Re: kern/41494

[Antti Kantee <pooka%NetBSD.org@localhost>]

The following reply was made to PR bin/41494; it has been noted by GNATS.

From: Antti Kantee <pooka%NetBSD.org@localhost>
To: Nicolas Joly <njoly%pasteur.fr@localhost>
Cc: gnats-bugs%NetBSD.org@localhost, a%zhtw.org.ru@localhost
Subject: Re: kern/41494
Date: Wed, 27 May 2009 19:13:26 +0300

 On Wed, May 27, 2009 at 05:23:01PM +0200, Nicolas Joly wrote:
 > For some reason the offset value in sysctlfs_node_read() exceed the
 > localbuf buffer size, leading to an out-of-bound access with
 > memcopy...
 > 
 > 635             memcpy(buf, localbuf + offset, xfer);
 > (gdb) p localbuf
 > $1 = "NetBSD", '\0' <repeats 8185 times>
 > (gdb) p offset
 > $2 = 8199
 > (gdb) p sizeof(localbuf)
 > $3 = 8192
 > (gdb) p xfer
 > $4 = 4096
 
 Uh oh ... ok, I think this is what happening:
 
         int xfer;
         xfer = MIN(*resid, strlen(localbuf) - offset);
 
 On i386 size_t (from strlen) is smaller than off_t (offset), hence
 it is widened to signed and the result the calculation is
 signed (64) - signed(64) ==> signed.  On amd64 where size_t is the
 same size as off_t, the calculation is unsigned - signed (converted
 to unsigned) ==> unsigned and hence *resid is always smaller when
 offset is larger than the file size without the trailing \n.
 
 .... *blink blink*.  I must confess I didn't see that one coming
 when I wrote the code.
 
 Try if this helps this:
 
 Index: sysctlfs.c
 ===================================================================
 RCS file: /cvsroot/src/usr.sbin/puffs/mount_sysctlfs/sysctlfs.c,v
 retrieving revision 1.10
 diff -p -u -r1.10 sysctlfs.c
 --- sysctlfs.c  18 Jan 2009 10:10:47 -0000      1.10
 +++ sysctlfs.c  27 May 2009 16:06:41 -0000
 @@ -627,9 +627,12 @@ sysctlfs_node_read(struct puffs_usermoun
                 return EISDIR;
  
         doprint(sfs, &pn->pn_po, localbuf, sizeof(localbuf));
 -       xfer = MIN(*resid, strlen(localbuf) - offset);
 +       if (strlen(localbuf) == offset)
 +               xfer = 0;
 +       else
 +               xfer = MIN(*resid, strlen(localbuf) - offset);
  
 -       if (xfer <= 0)
 +       if (xfer < 0)
                 return 0;
  
         memcpy(buf, localbuf + offset, xfer);