Re: old bug: mount_nfs path/name is limited to 88 chars

Xin Li wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA512
> 
> On 01/19/15 13:20, Garrett Cooper wrote:
> > On Jan 19, 2015, at 8:46, Brandon Allbery <allbery.b_at_gmail.com>
> > wrote:
> > 
> >> On Mon, Jan 19, 2015 at 10:44 AM, Anton Shterenlikht
> >> <mexas_at_bris.ac.uk> wrote:
> >> 
> >>> So perhaps changing MNAMELEN will break statfs(2) on -stable
> >>> too?
> >>> 
> >> 
> >> I believe the context there is not so much "-current is special",
> >> as "changing it for everyone is bad news" (and this would
> >> necessarily need to originate in -current).
> > 
> > A compat layer needs to be created for all of the affected
> > syscalls, and the change needs to be made. That’s it in a
> > nutshell.
> > 
> > Doing it in 11 makes sense since there is a compat layer for 10
> > now… if I knew all of the steps I would happily do them as annoys
> > me from time to time as well with the path length issue.
> 
> Compat layer may break applications in other funny ways and we
> probably have to return e.g. ENAMETOOLONG for legacy applications if
> the names are too long to fit into the buffer, but I don't see any
> easy solution either: I wish we have bumped it in 2003 when the
> struct
> receives its first big revamp by bumping all statistic fields to
> 64-bit.
> 
> I personally think we probably better create a new API and keep the
> existing one for (limited) compatibility.  For instance, it may be
> sensible to make f_mntfromname and f_mntonname fields variable length
> and have the caller pass a pointer and their length.  If we set an
> arbitrary limit, no matter it's 88, 256 or 4096, one will hit it some
> time.
> 
> BTW. If someone wants to change statfs(2), please make sure to create
> reverse-compatibility shims at the same time (see
> lib/libc/sys/fcntl.c
> for an example).  The statfs(2) API is used in a lot of places,
> especially fts(3), and breaking it either way (running new world with
> old kernel, or running old world with new kernel) would be a big pain
> to recover from.
> 
Solaris has statvfs(), which might be a starting point for a new API.
statfs(2) would have to be retained for compatibility with older binaries and
I think there would still be some breakage for cases where the mount path
does exceed MNAMELEN. All I can think of is that the mount path would have
to be truncated for the compatibility code.
(I don't know what kind of truncation would be preferred?
 Just chop it at 88 or do something like /sub1/.../mnt or 0 length?)

I think a lot of things will bump up against PATH_MAX, so making the length
that might be ok, although I like the new API having the buffer and its
length being passed in as arguments, so at least the syscall API doesn't
need to change again.

Some variant of doing all this is in projects/ino64 I think, since it
ends up changing assorted syscalls like stat(2) as well, along with
versioning the libc functions affected by the syscall changes. (I'd
guess fts(3) is one of them, but haven't looked.)

I doubt these changes could be MFC'd (others mention doing this for
FreeBSD11).

What could be done and maybe MFC'd is to simply allow
mounts with paths greater than 88, but truncate it to 88 for statfs(2).
{ I think this truncation will end up happening for the compatibility
  syscall code anyhow, even if a new statfs(2) is added, so maybe just let
  it happen, even if no new syscall is available. }
There was a patch floating around for this and doing this would only
impact mounts with paths > MNAMELEN. (mount_nfs could print a warning
for paths > MNAMELEN instead of failing the mount.)
Does this sound reasonable as a stop gap (and could it be MFC'd?)?

rick

> Cheers,
> - --
> Xin LI <delphij_at_delphij.net>    https://www.delphij.net/
> FreeBSD - The Power to Serve!           Live free or die
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2.1.1 (FreeBSD)
> 
> iQIcBAEBCgAGBQJUvanaAAoJEJW2GBstM+nsgoQP/0+Ulfmmmj9n9cWA0h1NxD3O
> a59aZXxbFtRu2SkIZprzFOL4TLnmyKEthFGmUgRSm7SH3GuaWVPBsgHTjSddYA/f
> vD3fBei780akT/higazmSENKR0eWqL0zENG8HJndQ0XLLcv8eeHZEFFhbUlYA4JU
> 4ArzoKEwsxiQ4jKB+KSFRU4QR4Raarljx6m4gQyXO6QAPEcyO035YH+bJlMSPjYg
> wcJZiZXmsRYsjZMhKDGeO5tIiD8R/UwOcMKsaQ/O+bwCgWtHFe5gLDKxGjMlLc9c
> NuhXE2/xlyBdAf3OHTlgr61dPmArQl0pyLXDUfd8K0s05FQ22bGHaSCT0FyNNG0J
> 55rnr30iDczVjQV1rTk9AwvsTJzACyaRrCV3zXGpxr86XwGFRta4ebMYZNuRXhdZ
> sLsTZWpIc3gnvOH4CiZHPmr1mbHoAY1RN9G23T5ENu2zYNJVozXhJ4NkoZkKzxUj
> b19qox4aKtG9QT7h5HU7R7Q64Sz2dYty8VD4OZ/azQrLGjVdI+2KUq6M3SAIRBro
> IAmGY62OQyz8aDVhr/Ap/N7GnV4suCZf08kgg3+oREU0a8QAxEHCDnNH4MJ9xz6v
> +2GpYWp0AuCFJ77XQC7IB6va1hK+PZnuOZ9yNVKTTadEwSk4GK2fQv6YwLjiyYKM
> PiOk31dmRfQB+3mqZ+Oj
> =a3fA
> -----END PGP SIGNATURE-----
>