Re: Possible bug in NFSv4 with krb5p security?

2013-03-25 Thread Andrey Simonenko 
On Wed, Feb 20, 2013 at 06:29:07PM -0500, Rick Macklem wrote:
 Andrey Simonnenko wrote:
  
  Another variant. This is a program that can be used for verifying
  correctness of the function, just change PWBUF_SIZE_* values and put
  some printfs to see when buffer is reallocated. sizehint is updated
  only when malloc() succeeded.
  
  -
  static int
  getpwnam_r_func(const char *name, uid_t *uidp)
  {
  #define PWBUF_SIZE_INI (2 * MAXLOGNAME + MAXPATHLEN + _PASSWORD_LEN)
  #define PWBUF_SIZE_INC 128
  
  static size_t sizehint = PWBUF_SIZE_INI;
  
  struct passwd pwd;
  struct passwd *pw;
  char *buf;
  size_t size;
  int error;
  char lname[MAXLOGNAME];
  char bufs[PWBUF_SIZE_INI];
  
  strncpy(lname, name, sizeof(lname));
  
  buf = bufs;
  size = sizeof(bufs);
  for (;;) {
  error = getpwnam_r(lname, pwd, buf, size, pw);
  if (buf != bufs)
  free(buf);
  if (pw != NULL) {
  *uidp = pw-pw_uid;
  return (GSS_S_COMPLETE);
  } else if (error != ERANGE || size  SIZE_MAX - PWBUF_SIZE_INC)
  return (GSS_S_FAILURE);
  if (size != sizehint)
  size = sizehint;
  else
  size += PWBUF_SIZE_INC;
  buf = malloc(size);
  if (buf == NULL)
  return (GSS_S_FAILURE);
  sizehint = size;
  }
  }
  
 All looks fine to me. (Before my mailer messed with the whitespace;-)
 
 Thanks, rick
 ps: I will commit it in April, unless someone else does so sooner.

I was thinking about this approach once again and made a conclusion that
it is wrong.  Using static buffer at first and then allocate memory for
next calls can cause slowdown, depends on number of entries and used
database backend of course.  The libc code for getpwnam() allocates
memory for the buffer and does not free it on exit from the function,

If the above written code or any of its modification is going to be
committed to the source base (by you or by some another committer),
then I ask and require to not write/mention my name and email address
neither in source file nor in commit log message.

Appropriate commit log message for the above written code can be the
following:

--
Since FreeBSD does not support sysconf(_SC_GETPW_R_SIZE_MAX), then allocate
a buffer of sufficient size for getpwnam_r() as it is suggested in EXAMPLES
of SUSv4 documentation for the getpwnam_r() function.
--

since this documentation has similar code.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org

Re: Possible bug in NFSv4 with krb5p security?

2013-02-20 Thread Andrey Simonenko 
On Tue, Feb 19, 2013 at 08:52:49PM -0500, Rick Macklem wrote:
  
  I cannot find how to get information about maximum buffer size for
  the getpwnam_r() function. This information should be returned by
  sysconf(_SC_GETPW_R_SIZE_MAX), but since it does not work on FreeBSD
  it is necessary to guess its size. Original value is 128 and it works
  for somebody, 1024 works for your environment, but it can fail for
  another environment.
  
  SUSv4 specifies Storage referenced by the structure is allocated from
  the memory provided with the buffer parameter, but then tells about
  groups
  in EXAMPLE for getpwnam_r() Note that sysconf(_SC_GETPW_R_SIZE_MAX)
  may
  return -1 if there is no hard limit on the size of the buffer needed
  to
  store all the groups returned.
  
  malloc() can give overhead, but that function can try to call
  getpwnam_r()
  with buffer allocated from stack and if getpwnam_r() failed with
  ERANGE
  use dynamically allocated buffer.
  
  #define PWBUF_SIZE_INI (2 * MAXLOGNAME + 2 * MAXPATHLEN +
  _PASSWORD_LEN + 1)
  #define PWBUF_SIZE_INC 128
  
  char bufs[2 * MAXLOGNAME + MAXPATHLEN + PASSWORD_LEN + 1 + 32];
  
  error = getpwnam_r(lname, pwd, bufs, sizeof(bufs), pw);
  if (pw != NULL) {
  *uidp = pw-pw_uid;
  return (GSS_S_COMPLETE);
  } else if (error != ERANGE)
  return (GSS_S_FAILURE);
  
  size = PWBUF_SIZE_INI;
  for (;;) {
  size += PWBUF_SIZE_INC;
  buf = malloc(size);
  if (buf == NULL)
  return (GSS_S_FAILURE);
  error = getpwnam_r(lname, pwd, buf, size, pw);
  free(buf);
  if (pw != NULL) {
  *uidp = pw-pw_uid;
  return (GSS_S_COMPLETE);
  } else {
  if (error == ERANGE 
  size = SIZE_MAX - PWBUF_SIZE_INC)
  continue;
  return (GSS_S_FAILURE);
  }
  }
 
 Just my opinion, but I think the above is a good approach.
 (ie. First trying a fairly large buffer on the stack that
  will succeed 99.99% of the time, but check for ERANGE and
  loop trying progressively larger malloc'd buffers until
  it stops reporting ERANGE.)
 
 I suspect the overheads behind getpwnam_r() are larger than
 the difference between using a buffer on the stack vs malloc,
 so I think it should use a fairly large buffer the first time.
 
 Personally, I might have coded it as a single do { } while(),
 with the first attempt in it, but that's just personal stylistic
 taste. (You can check for buf != bufs before doing a free() of it.)
 And, if you wanted to be clever, the code could use a static bufsiz_hint,
 which is set to the largest size needed sofar and that is used as
 the initial malloc size. That way it wouldn't loop as much for a
 site with huge passwd entries. (An entire bio in the GECOS field or ???)
 

Thanks for the review.

Another variant.  This is a program that can be used for verifying
correctness of the function, just change PWBUF_SIZE_* values and put
some printfs to see when buffer is reallocated.  sizehint is updated
only when malloc() succeeded.

-
#include sys/param.h
#include sys/limits.h

#include gssapi/gssapi.h

#include errno.h
#include limits.h
#include pwd.h
#include stdio.h
#include stdlib.h
#include string.h
#include unistd.h

static int
getpwnam_r_func(const char *name, uid_t *uidp)
{
#define PWBUF_SIZE_INI (2 * MAXLOGNAME + MAXPATHLEN + _PASSWORD_LEN)
#define PWBUF_SIZE_INC 128

static size_t sizehint = PWBUF_SIZE_INI;

struct passwd pwd;
struct passwd *pw;
char *buf;
size_t size;
int error;
char lname[MAXLOGNAME];
char bufs[PWBUF_SIZE_INI];

strncpy(lname, name, sizeof(lname));

buf = bufs;
size = sizeof(bufs);
for (;;) {
error = getpwnam_r(lname, pwd, buf, size, pw);
if (buf != bufs)
free(buf);
if (pw != NULL) {
*uidp = pw-pw_uid;
return (GSS_S_COMPLETE);
} else if (error != ERANGE || size  SIZE_MAX - PWBUF_SIZE_INC)
return (GSS_S_FAILURE);
if (size != sizehint)
size = sizehint;
else
size += PWBUF_SIZE_INC;
buf = malloc(size);
if (buf == NULL)
return (GSS_S_FAILURE);
sizehint = size;
}
}

int
main(void)
{
const char *str[] = { man, root, q, bin, NULL };
uid_t uid;
u_int i;

for (i = 0; str[i] != NULL; ++i) {
printf(%-20s\t, str[i]);
if (getpwnam_r_func(str[i], uid) == GSS_S_COMPLETE)
printf(%u\n, uid);
else
printf(not found\n);
}
return (0);
}
-
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to

Re: Possible bug in NFSv4 with krb5p security?

2013-02-19 Thread Andrey Simonenko 
On Tue, Feb 19, 2013 at 12:06:13AM +0800, Elias Martenson wrote:
 
 You were right, the problem was in pname_to_uid.c. In it, the following
 code can be found:
 
 char lname[MAXLOGNAME + 1], buf[1024];
 
 /* some code snipped for brevity... */
 
 getpwnam_r(lname, pwd, buf, sizeof(buf), pw);
 if (pw) {
 *uidp = pw-pw_uid;
 return (GSS_S_COMPLETE);
 } else {
 return (GSS_S_FAILURE);
 }
 
 As it turns out, the getpwnam_r() call fails with ERANGE (I had to check
 the return value from getpwnam_r() in order to determine this, as pw is set
 to NULL both if there was an error or if the user name can't be found).
 
 Now, increasing the size of buf to 1024 solved the problem, and now the
 lookup works correctly.
 
 I wrote a small test program that issued the same call to getpwnam_r() and
 it worked. Until I su'ed to root, and then it failed.
 
 It seems as though the buffer needs to be bigger if you're root. I have no
 idea why, but there you have it. Problem solved.

It can require bigger buffer, since root can get the pw_password field
in the struct passwd{}.

Since sysconf(_SC_GETPW_R_SIZE_MAX) does not work on FreeBSD, the buffer
for getpwnam_r() call should have at least (2 * MAXLOGNAME + 2 * MAXPATHLEN +
_PASSWORD_LEN + 1) bytes (it is unclear how much is required for pw_gecos).

This buffer can be dynamically reallocated until getpwnam_r() is not
return ERANGE error (the following code has not been compiled and verified):

#define PWBUF_SIZE_INI (2 * MAXLOGNAME + 2 * MAXPATHLEN + _PASSWORD_LEN + 1)
#define PWBUF_SIZE_INC 128

size = PWBUF_SIZE_INI;
for (;;) {
size += PWBUF_SIZE_INC;
buf = malloc(size);
if (buf == NULL)
return (GSS_S_FAILURE);
error = getpwnam_r(lname, pwd, buf, size, pw);
free(buf);
if (pw != NULL) {
*uidp = pw-pw_uid;
return (GSS_S_COMPLETE);
} else {
if (error == ERANGE 
size = SIZE_MAX - PWBUF_SIZE_INC)
continue;
return (GSS_S_FAILURE);
}
}
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org

Re: Possible bug in NFSv4 with krb5p security?

2013-02-19 Thread Andrey Simonenko 
On Tue, Feb 19, 2013 at 05:35:50PM +0800, Elias Martenson wrote:
 On 19 February 2013 17:31, Andrey Simonenko 
 si...@comsys.ntu-kpi.kiev.uawrote:
 
 It can require bigger buffer, since root can get the pw_password field
  in the struct passwd{}.
 
  Since sysconf(_SC_GETPW_R_SIZE_MAX) does not work on FreeBSD, the buffer
  for getpwnam_r() call should have at least (2 * MAXLOGNAME + 2 *
  MAXPATHLEN +
  _PASSWORD_LEN + 1) bytes (it is unclear how much is required for pw_gecos).
 
  This buffer can be dynamically reallocated until getpwnam_r() is not
  return ERANGE error (the following code has not been compiled and
  verified):
 
 
 Is this really a better solution than to aim high right away? A series of
 malloc() calls should certainly have much higher overhead than the previous
 stack-allocated solution.
 
 A better compromise would be to do the lookup in a separate function, that
 allocates the buffer using alloca() instead, yes?

I cannot find how to get information about maximum buffer size for
the getpwnam_r() function.  This information should be returned by
sysconf(_SC_GETPW_R_SIZE_MAX), but since it does not work on FreeBSD
it is necessary to guess its size.  Original value is 128 and it works
for somebody, 1024 works for your environment, but it can fail for
another environment.

SUSv4 specifies Storage referenced by the structure is allocated from
the memory provided with the buffer parameter, but then tells about groups
in EXAMPLE for getpwnam_r() Note that sysconf(_SC_GETPW_R_SIZE_MAX) may
return -1 if there is no hard limit on the size of the buffer needed to
store all the groups returned.

malloc() can give overhead, but that function can try to call getpwnam_r()
with buffer allocated from stack and if getpwnam_r() failed with ERANGE
use dynamically allocated buffer.

#define PWBUF_SIZE_INI (2 * MAXLOGNAME + 2 * MAXPATHLEN + _PASSWORD_LEN + 1)
#define PWBUF_SIZE_INC 128

char bufs[2 * MAXLOGNAME + MAXPATHLEN + PASSWORD_LEN + 1 + 32];

error = getpwnam_r(lname, pwd, bufs, sizeof(bufs), pw);
if (pw != NULL) {
*uidp = pw-pw_uid;
return (GSS_S_COMPLETE);
} else if (error != ERANGE)
return (GSS_S_FAILURE);

size = PWBUF_SIZE_INI;
for (;;) {
size += PWBUF_SIZE_INC;
buf = malloc(size);
if (buf == NULL)
return (GSS_S_FAILURE);
error = getpwnam_r(lname, pwd, buf, size, pw);
free(buf);
if (pw != NULL) {
*uidp = pw-pw_uid;
return (GSS_S_COMPLETE);
} else {
if (error == ERANGE 
size = SIZE_MAX - PWBUF_SIZE_INC)
continue;
return (GSS_S_FAILURE);
}
}
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org

Re: [CFT] Some updates to libc/rpc (second try)

2012-08-31 Thread Andrey Simonenko 
On Thu, Aug 30, 2012 at 02:37:17AM -0700, Garrett Cooper wrote:
  Detailed description of mistakes in these files and correct implementation:
 
  http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/165710
 
 A developer at $work (Isilon) developed a slightly simpler patch than
 that based on our custom 7.x sources recently to deal with concurrency
 issues in netconfig. I'll talk with a couple people to see whether or
 not the solution can be contributed back [after some polishing --
 maybe -- and further testing].

Can you post changes and corrected bugs in getnetconfig.c and getnetpath.c
in your (your $work) implementation.

This is the list of changes and corrected bugs in my implementation:

1. __nc_error() does not check return value from malloc() and can
pass NULL pointer to thr_setspecific().

2. setnetconfig() has a race condition with reference counter when
several threads call this function and only one thread successfully
opened database file, while other threads failed in this function.
If that one thread called endnetconfig(), then it can keep cached data
in memory, but all threads will not have opened handles.

3. getnetconfig() should return entries from /etc/netconfig file in
the same order as they are specified according to netconfig(5).
If several threads call getnetconfig() using different handlers,
then entries for each handler will be returned in random order.

4. getnetconfig() has a race condition that can cause NULL pointer
dereference when several threads call this function using one handler.

5. getnetconfig() allows to continue to get entries if database file has
invalid format, because it does not remember previous state.

6. endnetconfig() has a race condition with reference count and
can keep cached data, while all handlers are closed.

7. getnetconfigent() uses getnetconfig() and has the same mistakes,
also this function duplicates code from getnetconfig().

8. getnetconfig() and getnetconfigent() use too much memory for
entry data, each entry require ~1 kbytes of memory, while usually
only 50 bytes is needed.

9. parse_ncp() incorrectly parses flags in netconfig entry and allows
wrong combinations of flags, it does not allow spaces before entry,
does not check number of elements in each netconfig entry, does not
allow empty lines.

10. nc_sperror() is not optimal.

11. dup_ncp() is not optimal, allocates more memory than was used in
the original structure, call strcpy() several times instead of
calling memcpy() one time.

12. setnetpath() is not optimal, e.g. it calls setnetconfig() and then
calls endnetconfig().

13. getnetpath() uses getnetconfig() and getnetconfigent() and has
the same mistakes.

14. getnetpath() has race conditions when several threads call this
function using one handler, as a result there are memory leaks
and not synchronized access with modifications to data if the NETPATH
environment variable is set.

15. _get_next_token() is too complex, incorrectly understand \-sequences.

16. All functions do not specify error code in all possible cases,
so nc_sperror() and nc_perror() functions are useless.

Difference between netconfig.c vs getnetconfig.c and getnetpath.c:

1. __nc_error() was corrected, but its implementation is the same,
this is a standard implementation for thread-specific data handling.
nc_perror() was taken from getnetconfig.c, it cannot be written
in other way.

2. Some errors messages were taken from getnetconfig.c.

3. New nc_parse() (old parse_ncp()) was corrected and optimized a bit,
it just parses white space separated fields in a string.

4. Some variables and macro variables names were taken from getnetconfig.c.

5. All other functions and data structures were rewritten.

Additionally I corrected libc/include/reentrant.h, getnetconfig.3,
and getnetpath.3.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org

Re: [CFT] Some updates to libc/rpc (second try)

2012-08-31 Thread Andrey Simonenko 
On Fri, Aug 31, 2012 at 08:12:09AM -0400, John Baldwin wrote:
 On Friday, August 31, 2012 7:06:53 am Andrey Simonenko wrote:
  On Thu, Aug 30, 2012 at 02:37:17AM -0700, Garrett Cooper wrote:
Detailed description of mistakes in these files and correct 
 implementation:
   
http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/165710
   
   A developer at $work (Isilon) developed a slightly simpler patch than
   that based on our custom 7.x sources recently to deal with concurrency
   issues in netconfig. I'll talk with a couple people to see whether or
   not the solution can be contributed back [after some polishing --
   maybe -- and further testing].
  
  Can you post changes and corrected bugs in getnetconfig.c and getnetpath.c
  in your (your $work) implementation.
 
 There is a thread on threads@ with patches to make the API thread-safe
 that I believe came from Isilon.  It was posted in the last week or so,
 so it should be easy to find in the archives.
 

Thank you for this information.

I checked the logic of their changes.

Making all data per-thread is wrong from my point of view.

1. Several threads can call getnetconfig() using the same handler obtained
   by setnetconfig().  If each thread has own FILE pointer, then some
   getnetconfig() will crash a program since fgets() will be called for NULL
   FILE pointer.

2. One thread can get handler by setnetconfig() and pass this handler to
   another thread and getnetconfig() will crash a program.  This is the
   similar mistake, but getnetconfig() is not called in parallel.

3. Each thread has to open netconfig(5) database, parse its content every
   time for each getnetconfig() call since data is not cached.  This is slow.

There is one per-thread value, it is error code of the last failed function,
this value cannot be kept in handler, since nc_perror() and nc_sperror()
do not have handler argument.   Since printing error is expected in the
same thread when getnetconfig() failed (for example), I think this is Ok.
If error is print in another thread, then we simply will not see correct
error message, but program will not be crashed.

I just commented only per-thread idea of data in getnetconfig.c, I do not
compare my implementation, since their patch does not correct any other
mistake described and corrected in my PR.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org

Re: [CFT] Some updates to libc/rpc (second try)

2012-08-30 Thread Andrey Simonenko 
On Wed, Aug 29, 2012 at 09:57:37PM -0500, Pedro Giffuni wrote:
 
 This is rather critical stuff (libc) so I have no hurry and would
 like extensive testing before considering it for head.
 
 Please give it a try and report any issue.
 

Looks like that their getnetconfig.c and getnetpath.c have similar
mistakes as these files have in FreeBSD.

Detailed description of mistakes in these files and correct implementation:

http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/165710

Related PR:

http://www.freebsd.org/cgi/query-pr.cgi?pr=79683
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org

Comparison of the nfse compatibility mode with mountd and exports(5)

2012-07-18 Thread Andrey Simonenko 
On Sat, Jun 30, 2012 at 08:53:09PM -0400, Rick Macklem wrote:

 I haven't looked at Andrey's patch, but conceptually it sounds like
 the best approach. As I understand it, the problem with replacing
 mountd with nfse (at least in the FreeBSD source tree) is that nfse
 is not 100% backwards compatible with /etc/exports and, as such, is
 a POLA violation.

Since NFSE was mentioned in this thread and there were some opinions
about its compatibility with existent NFS exports configuration, I made
comparison of the nfse compatibility mode with mountd and exports(5):

http://nfse.sourceforge.net/COMPATIBILITY
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org

Re: Occassional permission denied in the middle of a large transfer over NFS

2012-07-10 Thread Andrey Simonenko 
This message contains only corrections to typos from my previous message.

On Mon, Jul 09, 2012 at 12:44:56PM +0300, Andrey Simonenko wrote:
 
 Here -alldirs means that /cdrom should be a file system.  As I remember
 this even worked before revision 1.85 of mountd/mountd.c, then mountd.c
 began to use nmount(2).  Now if one puts /cdrom in /etc/exports and
   ^^\
  /cdrom -alldirs

 Let's consider the same example but without the -alldirs options:
 
 /usr/local/export -maproot=root 1.2.3.4
 
 3. nfse in the native mode will export /usr if it is or will be a mount point:
 \
  /usr/local/export
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org

Re: Occassional permission denied in the middle of a large transfer over NFS

2012-07-09 Thread Andrey Simonenko 
Hello again,

On Sun, Jul 08, 2012 at 06:35:50PM +0100, Vincent Hoffman wrote:
 Replying to myself just as a record, I have tried nfse and I didnt get
 the permission denied at all.
 The only issue I had with it is that it strictly adheres to the syntax
 in exports(5) while mountd is a little more flexible.
 
 I had
 /usr/local/export -alldirs -maproot=root 85.xx.xx.xx
 
 /usr is the root of that filesystem
 
 mountd - allowed this but actually silently exports /usr (and all dirs
 below)
 
 nfse - didnt allow this, it needed to be the correct
 /usr  -alldirs -maproot=root 85.xx.xx.xx
 
 which is I guess a POLA violation but follows the documentation.
 this was using nfse in mountd compatibility mode. I havent played with
 its more advanced features.

The -alldirs option in the exports(5) file does not mean only
all directories.

Please read these lines from exports(5) DESCRIPTION:

 The second is to specify the pathname of the root of the file system fol-
 lowed by the -alldirs flag; this form allows the host(s) to mount at any
 point within the file system, including regular files if the -r option is
 used on mountd(8).

The -alldirs option specifies that this pathname must be the root (mount
point) of some file system.

And read these lines from EXAMPLES:

 The file system rooted at /cdrom will be exported read-only to the entire
 network 192.168.33.0/24, including all its subdirectories.  Since /cdrom
 is the conventional mountpoint for a CD-ROM device, this export will fail
 if no CD-ROM medium is currently mounted there since that line would then
 attempt to export a subdirectory of the root file system with the
 -alldirs option which is not allowed.  The -quiet option will then sup-
 press the error message for this condition that would normally be sys-
 logged.  As soon as an actual CD-ROM is going to be mounted, mount(8)
 will notify mountd(8) about this situation, and the /cdrom file system
 will be exported as intended.  Note that without using the -alldirs
 option, the export would always succeed.  While there is no CD-ROM medium
 mounted under /cdrom, it would export the (normally empty) directory
 /cdrom of the root file system instead.

Here -alldirs means that /cdrom should be a file system.  As I remember
this even worked before revision 1.85 of mountd/mountd.c, then mountd.c
began to use nmount(2).  Now if one puts /cdrom in /etc/exports and
there is no file system mounted on /cdrom, then the entire / will be
exported.

Just create /etc/exports with one line /cdrom -alldirs and try to
mount server:/ on another system, you will get access to the / of
the server.

The current version of mountd is not little more flexible, it does
not follow traditional -alldirs option's logic and does not follow
description of the -alldirs option in the exports(5) manual page, I guess
this is a POLA violation.

Now let's back to your example where /usr is the root of the file system:

/usr/local/export -alldirs -maproot=root 1.2.3.4

1. mountd exports /usr and all subdirectories under it for NFSv2/3 clients.
   Actually it has to export /usr/local/export only and all subdirectories
   under it only if /usr/local/export is or will be the root of some
   file system.

2. nfse in the compatibility mode sees that there is the -alldirs option
   and will export /usr/local/export and all subdirectories under it
   only if /usr/local/exports is or will be the root of some file system.

If one runs nfse -Ct ... for this file then its output will be:

configure: reading file /etc/exports
configure: converting configuration to native format

Pathname /usr/local/export
Export specifications:
-rw -sec sys -maproot 0:0:5 -host 1.2.3.4
Subdirectories for NFSv2/3:
/usr/local/export
-alldirs -host 1.2.3.4

Warning: subdirectories exports are insecure

This is output in nfse native format.  Using NFSE part in the kernel,
nfse verifies whether /usr/local/export is a mount point, whether file
system supports NFS, and only if everything is correct, it starts to
export it.  When file system is unmounted or when another file system
is mounted, then NFSE part in the kernel is informed by EVENTHANDLERs
vfs_mount_event/vfs_unmount_event and NFS server verifies whether it
still can export this file system (it can be unmounted or it can be
shadowed).  The userland utility nfse got information of VFS events
via kevents VQ_MOUNT/VQ_UNMOUNT and only the verifies using NFSE
part in the kernel whether some file system is still exported or can
become exported.

There is one implementation mistake when nfse -C ... is used.  The
nfse utility follows implementation mistake from mountd and verifies
what is a file system and what is not a file system using the f_mntonname
field in the struct mount{}.  This is wrong since parts of mount point
directory can be renamed and this is not reflected in the f_mntonname
value.  That's why I do not

Re: Occassional permission denied in the middle of a large transfer over NFS

2012-07-09 Thread Andrey Simonenko 
On Sun, Jul 08, 2012 at 07:48:11PM -0400, Rick Macklem wrote:

  Replying to myself just as a record, I have tried nfse and I didnt get
  the permission denied at all.
  The only issue I had with it is that it strictly adheres to the syntax
  in exports(5) while mountd is a little more flexible.
  
  I had
  /usr/local/export -alldirs -maproot=root 85.xx.xx.xx
  
  /usr is the root of that filesystem
  
  mountd - allowed this but actually silently exports /usr (and all dirs
  below)
  
 Not exactly correct. mountd exports the entire file system in the kernel
 for the NFS server, since exports can only be attached to the mount points
 in the kernel. However, when the client's mount protocol requests a mount
 file handle for anything other than /usr/local/export, it will refuse that.
 (Which means that to mount anything other than /usr/local/export, the client
  must maliciously guess the file handle for mounting.)
 
 Put another way, a non-malicious NFSv3 client will only be able to mount
 /usr/local/export. Robert Watson calls this an administrative control and
 feels that it is necessary.

According to the exports(5) manual page and this example (/usr is the mount
point and the -alldir option is given), this example means the following:
export /usr/local/export only if it is or will be a mount point and
administratively export all subdirectories under it for NFSv2/3 clients.
Good description of the -alldirs option is given in the EXAMPLES section
from exports(5) in paragraph about /cdrom -alldirs.

___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org

Re: Occassional permission denied in the middle of a large transfer over NFS

2012-07-02 Thread Andrey Simonenko 
On Sun, Jul 01, 2012 at 12:13:30PM +0100, Vincent Hoffman wrote:
 On 01/07/2012 01:53, Rick Macklem wrote:
 
  I haven't looked at Andrey's patch, but conceptually it sounds like
  the best approach. As I understand it, the problem with replacing
  mountd with nfse (at least in the FreeBSD source tree) is that nfse
  is not 100% backwards compatible with /etc/exports and, as such, is
  a POLA violation.
 Understood. Its far from a simple drop in replacement.

List of difference between nfse -C ... (compatible mode with mountd)
and mountd is given here:

http://lists.freebsd.org/pipermail/freebsd-fs/2012-June/014554.html

If we ignore absence of some obsolete options support and some command
line options, the rest of differences visible to a user will occur only
if one does not follow rules of exports(5) file format.

The native mode of nfse (nfs.exports(5) file format) is different
than the logic of mountd, just because using existent exports(5) file
format it is impossible to specify export of not mounted file system,
it is impossible to specify all export settings for one file system in
one line, etc.

Can you verify whether nfse compatible mode with mountd is really
compatible with exports(5) files on your systems using instructions
from this message (no installation or patching is required):

http://lists.freebsd.org/pipermail/freebsd-fs/2010-May/008421.html
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org

Re: mountd, rpc.lockd and rpc.statd patches for testing

2012-04-23 Thread Andrey Simonenko 
On Thu, Apr 19, 2012 at 08:44:37PM -0400, Rick Macklem wrote:
 Andrey Simonenko wrote:
  On Mon, May 30, 2011 at 04:56:02PM -0400, Rick Macklem wrote:
   Hi,
  
   I have patches for the mountd, rpc.statd and rpc.lockd daemons
   that are meant to keep them from failing when a dynamically
   selected port# is not available for some combination of
 udp,tcp X ipv4,ipv6
  
   If anyone would like to test these patches, they can be found
   at:
  http://people.freebsd.org/~rmacklem/mountd.patch
  statd.patch
  lockd.patch
  
   Although I think I got them correct, they are rather big and ugly.
  
  
  I have checked this update for mountd in 10-CURRENT and has two
  questions:
  
  1. What is the sense to try to use the same port number for all
  supported netconfigs if specific port number is not given in
  a command line option?
  
 Well, there was a discussion of this on one of the mailing lists
 at the time. I started with a much simpler patch that didn't try and
 make all 4 udp/tcp, ip4/ip6 combinations use the same port#, but
 others felt that was important. (Something about tracking what port#
 were in use, but I can't quite recall. If you want to know the reasoning,
 look for the thread that would have been shortly before the commit.)
 

I verified how mountd, rpc.lockd and rpc.statd worked on previous
version of FreeBSD.  On 6.2-PRERELEASE mountd, rpc.lockd and rpc.statd
did not have the -h option and all these programs used random port numbers
for all netconfigs.  When the -h option was added to mountd, rpc.lockd
and rpc.statd this logic was changed and random port was used for all
netconfigs and for all IP address if they are specified.  Obviously
everyone who used NFS after these changes got non working NFS several
times even for configurations without the -h option.

rpcbind(8) in FreeBSD does not allow multiple settings for the same
network ID, so wildcard address for each netconfig should be used.
As I understand this is a design decision and multiple settings for
one RPC service are refused in rpcbind/rpcb_svc_com.c:map_set().

Would all possible cases be checked before commiting support for
the -h option, may be that option and broken logic will not appear
in mountd, rpc.lockd and rpc.statd.  Now, everything works, but mountd
still can fail during startup (depends on activity of other network
programs of course).

  
  One comment for netconfig related functions usage. Each setnetconfig()
  call allocates memory and depending on implementation can use other
  resources, so endnetconfig() should be called before reusing netconfig
  handle.
  _
 Ok, I'll take a look someday. Since it happens a finite number of times,
 any leak should be bounded and, as such, shouldn't cause serious problems.
 However, I wasn't aware of the above and it should be fixed.
 

When I worked on my changes for NFS server, I verified implementation of
netconfig related function in RPC part of libc and not closed netconfig
handles results in memory leaks and one opened file descriptor for netconfig
database.  BTW there are several mistakes in that part of code, so I wrote
correct implementation in kern/165710 (kern category was autoassigned).
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org

Re: mountd, rpc.lockd and rpc.statd patches for testing

2012-04-20 Thread Andrey Simonenko 
On Thu, Apr 19, 2012 at 08:44:37PM -0400, Rick Macklem wrote:
 Andrey Simonenko wrote:
  
  1. What is the sense to try to use the same port number for all
  supported netconfigs if specific port number is not given in
  a command line option?
  
 Well, there was a discussion of this on one of the mailing lists
 at the time. I started with a much simpler patch that didn't try and
 make all 4 udp/tcp, ip4/ip6 combinations use the same port#, but
 others felt that was important. (Something about tracking what port#
 were in use, but I can't quite recall. If you want to know the reasoning,
 look for the thread that would have been shortly before the commit.)

That discussion was in stable@ mailing list under the subject
statd/lockd startup failure started at February 2011.  That discussion
had two reasons to use one port for all netconfigs: 1) tracking network
connections and 2) firewall configurations.

If specific port number is not given in a command line, then this port
number should be taken from rpcinfo output for example, so it should be
taken manually or by some script.  Anyway it will require some work to
obtain a port number before using it in tcpdump or in firewall settings.

I checked rpcinfo output for mountd on Solaris and NetBSD, on both
systems mountd can use different ports for different netconfigs.

  2. What is the sense of specifying specific IP addresses for mountd
  and
  similar RPC programs that do not have predefined port numbers?
  
 I'm not sure what you are asking here? (Are you referring to the -h
 command line option?)

Yes, about the -h command line option.

Such option works for nfsd, since it has predefined port number,
but it will not work correctly for other RPC programs when specific
port number is not given.  Bigger number of specific addresses given
in this option will increase probability that mountd will fail.
There are several attempts to select one random port number for all
netconfigs (and for all specified addresses), but these attempts do not
guaranty that mountd will not fail.

Several systems do not have -h like option for nfsd, mountd, etc.

Looks like that when this option was proposed for mountd, rpc.statd and
rpc.lockd it was not considered that using non wildcard address for RPC
programs with not predefined port numbers does not fit with the RPC port
mapper logic.  (BTW rpc.lockd uses random port numbers for all netconfigs
on 10-CURRENT and 9-STABLE).

If the -h option (address) is really needed, then I would require to
specify the -p option (port number) as well, at least one will know
port number and can use it in firewall settings, but specifying unused
port number is required for all combinations of netconfigs and addresses.
Otherwise, successful start of mountd depends on number of -h options and
network activity of other programs.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org

Re: mountd, rpc.lockd and rpc.statd patches for testing

2012-04-19 Thread Andrey Simonenko 
On Mon, May 30, 2011 at 04:56:02PM -0400, Rick Macklem wrote:
 Hi,
 
 I have patches for the mountd, rpc.statd and rpc.lockd daemons
 that are meant to keep them from failing when a dynamically
 selected port# is not available for some combination of
   udp,tcp X ipv4,ipv6
 
 If anyone would like to test these patches, they can be found
 at:
http://people.freebsd.org/~rmacklem/mountd.patch
statd.patch
lockd.patch
 
 Although I think I got them correct, they are rather big and ugly.
 

I have checked this update for mountd in 10-CURRENT and has two questions:

1. What is the sense to try to use the same port number for all
   supported netconfigs if specific port number is not given in
   a command line option?

2. What is the sense of specifying specific IP addresses for mountd and
   similar RPC programs that do not have predefined port numbers?

--

One comment for netconfig related functions usage.  Each setnetconfig()
call allocates memory and depending on implementation can use other
resources, so endnetconfig() should be called before reusing netconfig
handle.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org

Re: -ffast-math in Ports and wrong generated code

2012-04-05 Thread Andrey Simonenko 
On Wed, Apr 04, 2012 at 09:45:25AM -0500, Pedro Giffuni wrote:
 On 04/04/12 04:29, Andrey Simonenko wrote:
  On Tue, Apr 03, 2012 at 06:43:00AM -0700, Steve Kargl wrote:
  On Tue, Apr 03, 2012 at 02:21:11PM +0300, Andrey Simonenko wrote:
  I use one port from the Ports Collection, that works with FP.  Having
  reinstalled it (its version was not changed) I noticed that it started
  to work incorrectly.  After debugging and disassembling its code I found
  out that the -ffast-math option used for building was the result of
  wrongly generated code (I did not specify this option in /etc/make.conf).
 
  At least finite() function call was eliminated from the result Assembler
  code when -ffast-math option is used, tested on 9.0-STABLE and 
  10.0-CURRENT.
 
  Example test source code and generated code under 9.0-STABLE on amd64
  by gcc from the base system:
 
  -
  #includemath.h
  #includestdio.h
 
  void
  check_finite(double x)
  {
printf(%d\n, finite(x));
  }
  -
 
  % gcc -Wall -O2 -S finite.c
  -
  check_finite:
  .LFB3:
subq$8, %rsp
  .LCFI0:
callfinite  -- call to finite()
movl$.LC0, %edi
movl%eax, %esi
addq$8, %rsp
xorl%eax, %eax
jmp printf
  .LFE3:
.size   check_finite, .-check_finite
  -
 
  % gcc -Wall -O2 -ffast-math -S finite.c
  -
  check_finite:
  .LFB3:
xorl%esi, %esi  -- fake result from finite()
movl$.LC0, %edi
xorl%eax, %eax
jmp printf
  .LFE3:
.size   check_finite, .-check_finite
  -
 
  Can somebody comment this?
  Read the man page for gcc.  With --fast-math,
  gcc assumes that the result of any FP operation
  is finite.  So, the function call to finite()
  is eliminated as it is always true.
  Looks like that I was misunderstood.  I did not ask why finite() was
  eliminated, I asked why fake result from finite() is wrong.  Obviously
  that -ffast-math can optimize FP arithmetics and as a result some functions
  can be eliminated.  The problem is not respecting IEEE specifications for
  FP, the problem is wrongly generated code when -ffast-math is used.
 
  Actually there is a bug in GCC used in the base system.  There was made
  a change to builtins.c from gcc in revision 1.12 [1] and as a result gcc
  started to eliminate finite() function calls with -ffinite-math-only.
 
  ...
 
  After this change the corresponding Assembler code for my test file is:
 
  % gcc -Wall -O2 -ffast-math -S finite.c
  -
  check_finite:
  .LFB3:
  movl$1, %esi-- fake result from finite()
  movl$.LC0, %edi
  xorl%eax, %eax
  jmp printf
  .LFE3:
  .size   check_finite, .-check_finite
  -
 
  What do you think?  If there is no objections, I'll create PR.
 
  [1] 
  http://www.freebsd.org/cgi/cvsweb.cgi/src/contrib/gcc/builtins.c.diff?r1=1.11;r2=1.12
  ___
 
 The SVN commit
 
 http://svnweb.freebsd.org/base?view=revisionrevision=228756
 
 will point you to this:
 
 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=28796
 
 and we are keeping consistency with both upstream and Apple's gcc.

Well, I've found exact commit to upstream gcc source tree that
corrects the bug described above:

http://gcc.gnu.org/viewcvs/trunk/gcc/builtins.c?r1=117751r2=117929

I think that nobody even tried to run my test program, so I'll post
here results from my system (I did not post this test before just to
make the post brief):

% uname -omr
FreeBSD 9.0-STABLE amd64

% cc -v
Using built-in specs.
Target: amd64-undermydesk-freebsd
Configured with: FreeBSD/amd64 system compiler
Thread model: posix
gcc version 4.2.1 20070831 patched [FreeBSD]

% cat finite.c
#include math.h
#include stdio.h

void
check_finite(double x)
{
printf( %d , finite(x));
}

int
main(void)
{
double x;

for (x = -2; x  2; x += 0.5) {
printf(%.1f, x);
check_finite(x);
}
printf(\n);
return (0);
}

% cc -Wall -O2 finite.c -lm
% ./a.out
-2.0 1 -1.5 1 -1.0 1 -0.5 1 0.0 1 0.5 1 1.0 1 1.5 1 

% cc -Wall -O2 -ffinite-math-only finite.c -lm
% ./a.out
-2.0 0 -1.5 0 -1.0 0 -0.5 0 0.0 0 0.5 0 1.0 0 1.5 0 

Should not both results be the same?
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org

Re: -ffast-math in Ports and wrong generated code

2012-04-04 Thread Andrey Simonenko 
On Tue, Apr 03, 2012 at 06:43:00AM -0700, Steve Kargl wrote:
 On Tue, Apr 03, 2012 at 02:21:11PM +0300, Andrey Simonenko wrote:
  
  I use one port from the Ports Collection, that works with FP.  Having
  reinstalled it (its version was not changed) I noticed that it started
  to work incorrectly.  After debugging and disassembling its code I found
  out that the -ffast-math option used for building was the result of
  wrongly generated code (I did not specify this option in /etc/make.conf).
  
  At least finite() function call was eliminated from the result Assembler
  code when -ffast-math option is used, tested on 9.0-STABLE and 10.0-CURRENT.
  
  Example test source code and generated code under 9.0-STABLE on amd64
  by gcc from the base system:
  
  -
  #include math.h
  #include stdio.h
  
  void
  check_finite(double x)
  {
  printf(%d\n, finite(x));
  }
  -
  
  % gcc -Wall -O2 -S finite.c
  -
  check_finite:
  .LFB3:
  subq$8, %rsp
  .LCFI0:
  callfinite  -- call to finite()
  movl$.LC0, %edi
  movl%eax, %esi
  addq$8, %rsp
  xorl%eax, %eax
  jmp printf
  .LFE3:
  .size   check_finite, .-check_finite
  -
  
  % gcc -Wall -O2 -ffast-math -S finite.c
  -
  check_finite:
  .LFB3:
  xorl%esi, %esi  -- fake result from finite()
  movl$.LC0, %edi
  xorl%eax, %eax
  jmp printf
  .LFE3:
  .size   check_finite, .-check_finite
  -
  
  Can somebody comment this?
 
 Read the man page for gcc.  With --fast-math,
 gcc assumes that the result of any FP operation 
 is finite.  So, the function call to finite()
 is eliminated as it is always true. 

Looks like that I was misunderstood.  I did not ask why finite() was
eliminated, I asked why fake result from finite() is wrong.  Obviously
that -ffast-math can optimize FP arithmetics and as a result some functions
can be eliminated.  The problem is not respecting IEEE specifications for
FP, the problem is wrongly generated code when -ffast-math is used.

Actually there is a bug in GCC used in the base system.  There was made
a change to builtins.c from gcc in revision 1.12 [1] and as a result gcc
started to eliminate finite() function calls with -ffinite-math-only.

The true result from finite() is non-zero value, but GCC generated always
false value, so any program that uses finite() and has -ffinite-math-only
works incorrectly if it was built by this version of gcc.

Here is the correction for builtins.c:

--- builtins.c.orig 2012-01-06 14:50:41.0 +0200
+++ builtins.c  2012-04-04 10:27:23.0 +0300
@@ -8738,7 +8738,7 @@ fold_builtin_classify (tree fndecl, tree
 case BUILT_IN_FINITE:
   if (!HONOR_NANS (TYPE_MODE (TREE_TYPE (arg)))
   !HONOR_INFINITIES (TYPE_MODE (TREE_TYPE (arg
-   return omit_one_operand (type, integer_zero_node, arg);
+   return omit_one_operand (type, integer_one_node, arg);
 
   if (TREE_CODE (arg) == REAL_CST)
{

After this change the corresponding Assembler code for my test file is:

% gcc -Wall -O2 -ffast-math -S finite.c
-
check_finite:
.LFB3:
movl$1, %esi-- fake result from finite()
movl$.LC0, %edi
xorl%eax, %eax
jmp printf
.LFE3:
.size   check_finite, .-check_finite
-

What do you think?  If there is no objections, I'll create PR.

[1] 
http://www.freebsd.org/cgi/cvsweb.cgi/src/contrib/gcc/builtins.c.diff?r1=1.11;r2=1.12
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org

-ffast-math in Ports and wrong generated code

2012-04-03 Thread Andrey Simonenko 
Hello,

I use one port from the Ports Collection, that works with FP.  Having
reinstalled it (its version was not changed) I noticed that it started
to work incorrectly.  After debugging and disassembling its code I found
out that the -ffast-math option used for building was the result of
wrongly generated code (I did not specify this option in /etc/make.conf).

At least finite() function call was eliminated from the result Assembler
code when -ffast-math option is used, tested on 9.0-STABLE and 10.0-CURRENT.

Example test source code and generated code under 9.0-STABLE on amd64
by gcc from the base system:

-
#include math.h
#include stdio.h

void
check_finite(double x)
{
printf(%d\n, finite(x));
}
-

% gcc -Wall -O2 -S finite.c
-
check_finite:
.LFB3:
subq$8, %rsp
.LCFI0:
callfinite  -- call to finite()
movl$.LC0, %edi
movl%eax, %esi
addq$8, %rsp
xorl%eax, %eax
jmp printf
.LFE3:
.size   check_finite, .-check_finite
-

% gcc -Wall -O2 -ffast-math -S finite.c
-
check_finite:
.LFB3:
xorl%esi, %esi  -- fake result from finite()
movl$.LC0, %edi
xorl%eax, %eax
jmp printf
.LFE3:
.size   check_finite, .-check_finite
-

Can somebody comment this?
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org

Re: [head tinderbox] failure on ia64/ia64

2011-02-01 Thread Andrey Simonenko 
On Mon, Jan 31, 2011 at 04:56:06PM -0800, Marcel Moolenaar wrote:
 
 Take the statement at line 116 for example:
   *((int *)CMSG_DATA(cmsg)) = fd;
 
 We're effectively casting from a (char *) to a (int *) and then doing
 a 32-bit access (write). The easy fix (casting through (void *) is not
 possible, because you cannot guarantee that the address is properly
 aligned. cmsg points to memory set aside by the following local
 variable:
   unsigned char ctrl[CMSG_SPACE(sizeof(fd))];
 
 There's no guarantee that the compiler will align the character array
 at a 32-bit boundary (though in practice it seems to be). I have seen
 this kind of construct fail on ARM and PowerPC for example.
 

Why not to use such declaration:

union {
struct cmsghdr cm;
char ctrl[CMSG_SPACE(sizeof(fd))];
} control_un;

At least this is necessary to satisfy that CMSG_FIRSTHDR() will give
address of correctly aligned struct cmsghdr{}.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org