nfs pathconf

2013-03-26 Thread Ted Unangst
After an absence of 9 years, I make my triumphant return to sys/nfs.
There are some silly programs out there (looking at you boost) that
actually use pathconf instead of just hard coding 1024 for max path
length. If you run them from nfs, fireworks ensue.

Here's a pathconf implementation for nfs, copied mostly from ufs. Even
if some of these values are lies, they're better than nothing. My
theory is that the values you care about (NAME_MAX, PATH_MAX) are
going to be right, and the values you don't (REC_MAX_XFER_SIZE) are
harmless even if incorrect.

Index: nfs_vnops.c
===
RCS file: /cvs/src/sys/nfs/nfs_vnops.c,v
retrieving revision 1.140
diff -u -p -r1.140 nfs_vnops.c
--- nfs_vnops.c 17 Nov 2012 22:28:26 -  1.140
+++ nfs_vnops.c 26 Mar 2013 07:04:53 -
@@ -61,6 +61,7 @@
 #include sys/hash.h
 #include sys/queue.h
 #include sys/specdev.h
+#include sys/unistd.h
 
 #include uvm/uvm_extern.h
 
@@ -2937,19 +2938,70 @@ loop:
 
 /*
  * Return POSIX pathconf information applicable to nfs.
- *
- * The NFS V2 protocol doesn't support this, so just return EINVAL
- * for V2.
  */
 /* ARGSUSED */
 int
 nfs_pathconf(void *v)
 {
-#if 0
struct vop_pathconf_args *ap = v;
-#endif
+   struct nfsmount *nmp = VFSTONFS(ap-a_vp-v_mount);
+   int error = 0;
 
-   return (EINVAL);
+   switch (ap-a_name) {
+   case _PC_LINK_MAX:
+   *ap-a_retval = LINK_MAX;
+   break;
+   case _PC_NAME_MAX:
+   *ap-a_retval = NAME_MAX;
+   break;
+   case _PC_PATH_MAX:
+   *ap-a_retval = PATH_MAX;
+   break;
+   case _PC_PIPE_BUF:
+   *ap-a_retval = PIPE_BUF;
+   break;
+   case _PC_CHOWN_RESTRICTED:
+   *ap-a_retval = 1;
+   break;
+   case _PC_NO_TRUNC:
+   *ap-a_retval = 1;
+   break;
+   case _PC_PRIO_IO:
+   *ap-a_retval = 0;
+   break;
+   case _PC_SYNC_IO:
+   *ap-a_retval = 0;
+   break;
+   case _PC_ALLOC_SIZE_MIN:
+   *ap-a_retval = NFS_FABLKSIZE;
+   break;
+   case _PC_FILESIZEBITS:
+   *ap-a_retval = 64;
+   break;
+   case _PC_REC_INCR_XFER_SIZE:
+   *ap-a_retval = min(nmp-nm_rsize, nmp-nm_wsize);
+   break;
+   case _PC_REC_MAX_XFER_SIZE:
+   *ap-a_retval = -1; /* means ``unlimited'' */
+   break;
+   case _PC_REC_MIN_XFER_SIZE:
+   *ap-a_retval = min(nmp-nm_rsize, nmp-nm_wsize);
+   break;
+   case _PC_REC_XFER_ALIGN:
+   *ap-a_retval = PAGE_SIZE;
+   break;
+   case _PC_SYMLINK_MAX:
+   *ap-a_retval = MAXPATHLEN;
+   break;
+   case _PC_2_SYMLINKS:
+   *ap-a_retval = 1;
+   break;
+   default:
+   error = EINVAL;
+   break;
+   }
+
+   return (error);
 }
 
 /*




Re: nfs pathconf

2013-03-26 Thread Bob Beck
Well, you're right about one thing - the comment there says that it should
just return EINVAL for nfs v2 - and I think it should - but that code returns
EINVAL for v3 - and that's wrong.  We have server side support for this in v3
and what we should probably be doing is actually doing the rpc call to the v3
server in the v3 case and returning what it returns - and doing EINVAL
in the v2 case.


On Tue, Mar 26, 2013 at 1:12 AM, Ted Unangst t...@tedunangst.com wrote:
 After an absence of 9 years, I make my triumphant return to sys/nfs.
 There are some silly programs out there (looking at you boost) that
 actually use pathconf instead of just hard coding 1024 for max path
 length. If you run them from nfs, fireworks ensue.

 Here's a pathconf implementation for nfs, copied mostly from ufs. Even
 if some of these values are lies, they're better than nothing. My
 theory is that the values you care about (NAME_MAX, PATH_MAX) are
 going to be right, and the values you don't (REC_MAX_XFER_SIZE) are
 harmless even if incorrect.

 Index: nfs_vnops.c
 ===
 RCS file: /cvs/src/sys/nfs/nfs_vnops.c,v
 retrieving revision 1.140
 diff -u -p -r1.140 nfs_vnops.c
 --- nfs_vnops.c 17 Nov 2012 22:28:26 -  1.140
 +++ nfs_vnops.c 26 Mar 2013 07:04:53 -
 @@ -61,6 +61,7 @@
  #include sys/hash.h
  #include sys/queue.h
  #include sys/specdev.h
 +#include sys/unistd.h

  #include uvm/uvm_extern.h

 @@ -2937,19 +2938,70 @@ loop:

  /*
   * Return POSIX pathconf information applicable to nfs.
 - *
 - * The NFS V2 protocol doesn't support this, so just return EINVAL
 - * for V2.
   */
  /* ARGSUSED */
  int
  nfs_pathconf(void *v)
  {
 -#if 0
 struct vop_pathconf_args *ap = v;
 -#endif
 +   struct nfsmount *nmp = VFSTONFS(ap-a_vp-v_mount);
 +   int error = 0;

 -   return (EINVAL);
 +   switch (ap-a_name) {
 +   case _PC_LINK_MAX:
 +   *ap-a_retval = LINK_MAX;
 +   break;
 +   case _PC_NAME_MAX:
 +   *ap-a_retval = NAME_MAX;
 +   break;
 +   case _PC_PATH_MAX:
 +   *ap-a_retval = PATH_MAX;
 +   break;
 +   case _PC_PIPE_BUF:
 +   *ap-a_retval = PIPE_BUF;
 +   break;
 +   case _PC_CHOWN_RESTRICTED:
 +   *ap-a_retval = 1;
 +   break;
 +   case _PC_NO_TRUNC:
 +   *ap-a_retval = 1;
 +   break;
 +   case _PC_PRIO_IO:
 +   *ap-a_retval = 0;
 +   break;
 +   case _PC_SYNC_IO:
 +   *ap-a_retval = 0;
 +   break;
 +   case _PC_ALLOC_SIZE_MIN:
 +   *ap-a_retval = NFS_FABLKSIZE;
 +   break;
 +   case _PC_FILESIZEBITS:
 +   *ap-a_retval = 64;
 +   break;
 +   case _PC_REC_INCR_XFER_SIZE:
 +   *ap-a_retval = min(nmp-nm_rsize, nmp-nm_wsize);
 +   break;
 +   case _PC_REC_MAX_XFER_SIZE:
 +   *ap-a_retval = -1; /* means ``unlimited'' */
 +   break;
 +   case _PC_REC_MIN_XFER_SIZE:
 +   *ap-a_retval = min(nmp-nm_rsize, nmp-nm_wsize);
 +   break;
 +   case _PC_REC_XFER_ALIGN:
 +   *ap-a_retval = PAGE_SIZE;
 +   break;
 +   case _PC_SYMLINK_MAX:
 +   *ap-a_retval = MAXPATHLEN;
 +   break;
 +   case _PC_2_SYMLINKS:
 +   *ap-a_retval = 1;
 +   break;
 +   default:
 +   error = EINVAL;
 +   break;
 +   }
 +
 +   return (error);
  }

  /*





Re: nfs pathconf

2013-03-26 Thread Theo de Raadt
 and doing EINVAL in the v2 case.

Which won't solve the problem described in his mail.



Re: nfs pathconf

2013-03-26 Thread Philip Guenther
On Tue, Mar 26, 2013 at 10:53 AM, Bob Beck b...@obtuse.com wrote:
 Well, you're right about one thing - the comment there says that it should
 just return EINVAL for nfs v2 - and I think it should - but that code returns
 EINVAL for v3 - and that's wrong.  We have server side support for this in v3
 and what we should probably be doing is actually doing the rpc call to the v3
 server in the v3 case and returning what it returns -

Sure, though nfsv3's pathconf call only supports a small subset of
pathconf() values:

struct nfsv3_pathconf {
u_int32_t pc_linkmax;
u_int32_t pc_namemax;
u_int32_t pc_notrunc;
u_int32_t pc_chownrestricted;
u_int32_t pc_caseinsensitive;
u_int32_t pc_casepreserving;
};


 and doing EINVAL in the v2 case.

I'm with tedu and Theo on this: giving best guess answers will provide
a better result than returning EINVAL.


Philip Guenther



Re: nfs pathconf

2013-03-26 Thread Bob Beck
On Tue, Mar 26, 2013 at 11:58 AM, Theo de Raadt dera...@cvs.openbsd.org wrote:
 and doing EINVAL in the v2 case.

 Which won't solve the problem described in his mail.

Of course it will - in the NFS v3 case, and in theory you'll be
getting what the server supports.

I don't think we should go outside the nfs v2 spec and return values
we hope are ok in the nfs v2 case - remember here we're returning
the client values and hoping the server can support it. So what you're
saying is that it's better to return a potentially unsafe value, and
have filenames that a potential server might not handle?  I'm not
buying that.  in that special case of I'm running some software that
does this, I'd rather realize I'm running it on v2 that doesn't
support it by having it fail than just using the client side value and
hope like heck it works out.  Presumably, if I'm running v2, I'm
doing it for specefic reasons that v2 is better for and I know what
I'm doing.  I'd rather not have the client start to do v3 like things
on v2 just so things might work. - at that point I should be
noticing the error, and the fix is mount it v3



Re: nfs pathconf

2013-03-26 Thread Ted Unangst
On Tue, Mar 26, 2013 at 12:51, Bob Beck wrote:
 On Tue, Mar 26, 2013 at 11:58 AM, Theo de Raadt dera...@cvs.openbsd.org
 wrote:
 and doing EINVAL in the v2 case.

 Which won't solve the problem described in his mail.
 
 Of course it will - in the NFS v3 case, and in theory you'll be
 getting what the server supports.
 
 I don't think we should go outside the nfs v2 spec and return values
 we hope are ok in the nfs v2 case - remember here we're returning
 the client values and hoping the server can support it. So what you're
 saying is that it's better to return a potentially unsafe value, and
 have filenames that a potential server might not handle?  I'm not
 buying that.  in that special case of I'm running some software that

Well, there's a certain amount of stupidity in pathconf already.
PATH_MAX could cross a mount point and then become something else
entirely. (Actually, that's an argument for factoring it up and
handling it above the fs layer, but that's another diff.)

 does this, I'd rather realize I'm running it on v2 that doesn't
 support it by having it fail than just using the client side value and
 hope like heck it works out.  Presumably, if I'm running v2, I'm
 doing it for specefic reasons that v2 is better for and I know what
 I'm doing.  I'd rather not have the client start to do v3 like things
 on v2 just so things might work. - at that point I should be
 noticing the error, and the fix is mount it v3

Maybe you're using amd? hahaha. I don't think the nfs v2 spec demands
that pathconf not work, it just doesn't support it because it didn't
exist.

Let me explain my philosophy towards pathconf. It's like those
configure scripts that check to see if you have a working version of
strcpy. If you don't, you are so utterly boned you'll find out soon
enough. If the nfs server isn't going to let you create a 255
character name, you'll find out soon enough. pathconf is one of those
design by committee anti-portability features. It's silly it's even in
the kernel. It should be a libc stub that whispers sweet nothings in
your ear.






Re: nfs pathconf

2013-03-26 Thread Bob Beck
 Let me explain my philosophy towards pathconf. It's like those
 configure scripts that check to see if you have a working version of
 strcpy. If you don't, you are so utterly boned you'll find out soon
 enough. If the nfs server isn't going to let you create a 255
 character name, you'll find out soon enough. pathconf is one of those
 design by committee anti-portability features. It's silly it's even in
 the kernel. It should be a libc stub that whispers sweet nothings in
 your ear.

So, does that make the case for fixing this in VOP_PATHCONF instead?

Call the underlying filesystem call if it's there? and if not return the
something sane there?  then we have our something defaultly sane
shit in one place?

Just thinking outloud...



Re: nfs pathconf

2013-03-26 Thread Theo de Raadt
  Let me explain my philosophy towards pathconf. It's like those
  configure scripts that check to see if you have a working version of
  strcpy. If you don't, you are so utterly boned you'll find out soon
  enough. If the nfs server isn't going to let you create a 255
  character name, you'll find out soon enough. pathconf is one of those
  design by committee anti-portability features. It's silly it's even in
  the kernel. It should be a libc stub that whispers sweet nothings in
  your ear.
 
 So, does that make the case for fixing this in VOP_PATHCONF instead?
 
 Call the underlying filesystem call if it's there? and if not return the
 something sane there?  then we have our something defaultly sane
 shit in one place?

For some of the variables, you don't need to walk the path.  For others,
you do.  Not enough axe murderers prowling the parking lots before austin
group meetings.