linuxolator problem on i386

[Boris Samorodov]

On Wed, 19 Sep 2007 13:15:57 +0200 Roman Divacky wrote:

> > There is nothing wrong with 4084. The magic 4096 is the
> > answer. I.e. the first case (when a buffer is equal to that
> > has been really read) is wrongly processed.
> > 
> > Ex. in C strings ends with 0x0. If a buffer 4096 bytes long was
> > assigned a string 4096 bytes long then the 0x0 will become a 4097
> > byte. And this string will cause a fault of the program.
> > 
> > PS. I don't say that I gave a strict example of the case. Of cause it
> > may be something else. But definitely there is a bug in processing of
> > 4096 bytes log strings by our linuxulator code (somewhere at the
> > linux_getdents area, linux_getdents64 used by linux_base-fc4 is not
> > affected by _at least_ 4096 bytes long strings).

> 1) what makes you think the 4096 buffer is wrongly processed? I still dont
> see it

>From ftp://ftp.ipt.ru/pub/linux/fc4.dump.txt (fc4, good start):
-----
 12988 hlds_i686 NAMI  "/usr/home/hlds/1.6/./cstrike/sound/weapons"
 12988 hlds_i686 RET   linux_open 11/0xb
 12988 hlds_i686 CALL  linux_fstat64(0xb,0xbfbecd9c,0x2820cff4)
 12988 hlds_i686 RET   linux_fstat64 0
 12988 hlds_i686 CALL  linux_fcntl64(0xb,0x2,0x1)
 12988 hlds_i686 RET   linux_fcntl64 0
 12988 hlds_i686 CALL  linux_getdents64(0xb,0x80f4f08,0x1000)
 12988 hlds_i686 RET   linux_getdents64 4084/0xff4
 12988 hlds_i686 CALL  linux_getdents64(0xb,0x80f4f08,0x1000)
 12988 hlds_i686 RET   linux_getdents64 4064/0xfe0
 12988 hlds_i686 CALL  linux_getdents64(0xb,0x80f4f08,0x1000)
 12988 hlds_i686 RET   linux_getdents64 4068/0xfe4
 12988 hlds_i686 CALL  linux_getdents64(0xb,0x80f4f08,0x1000)
 12988 hlds_i686 RET   linux_getdents64 492/0x1ec
 12988 hlds_i686 CALL  linux_getdents64(0xb,0x80f4f08,0x1000)
 12988 hlds_i686 RET   linux_getdents64 0
 12988 hlds_i686 CALL  close(0xb)
 12988 hlds_i686 RET   close 0
 12988 hlds_i686 CALL  linux_stat64(0xbfbecf3c,0xbfbecdfc,0x2820cff4)
 12988 hlds_i686 NAMI  "/compat/linux/usr/home/hlds/1.6/./valve/sound/weapons/reload1.wav"
-----
There is no 4096 bytes reading.
And from ftp://ftp.ipt.ru/pub/linux/hldc.kdump.txt:
-----
   937 hlds_i686 NAMI  "/compat/linux/usr/home/hlds/1.6/./cstrike/sound/weapons"
   937 hlds_i686 NAMI  "/usr/home/hlds/1.6/./cstrike/sound/weapons"
   937 hlds_i686 RET   linux_open 9
   937 hlds_i686 CALL  linux_fstat64(0x9,0xbfbecdac,0x281f1ff4)
   937 hlds_i686 RET   linux_fstat64 0
   937 hlds_i686 CALL  linux_fcntl64(0x9,0x2,0x1)
   937 hlds_i686 RET   linux_fcntl64 0
   937 hlds_i686 CALL  linux_getdents(0x9,0x8da2f34,0x1000)
   937 hlds_i686 RET   linux_getdents 4096/0x1000
   937 hlds_i686 CALL  linux_getdents(0x9,0x8da2f34,0x1000)
   937 hlds_i686 RET   linux_getdents 4064/0xfe0
   937 hlds_i686 CALL  linux_getdents(0x9,0x8da2f34,0x1000)
   937 hlds_i686 RET   linux_getdents 1588/0x634
   937 hlds_i686 CALL  linux_getdents(0x9,0x8da2f34,0x1000)
   937 hlds_i686 RET   linux_getdents 0
   937 hlds_i686 CALL  linux_open(0x281d7f05,0x902,0)
   937 hlds_i686 NAMI  "/compat/linux/dev/tty"
   937 hlds_i686 NAMI  "/dev/tty"
   937 hlds_i686 RET   linux_open 10/0xa
   937 hlds_i686 CALL  writev(0xa,0xbfbec730,0x7)
   937 hlds_i686 GIO   fd 10 wrote 86 bytes
       "*** glibc detected *** ./hlds_i686: double free or corruption (!prev):\
	 0x08da2f18 ***
       "
-----

There is 4096 bytes reading, core.

> 2) the getdents64 and getdents use exactly the same code :) the fact is
> that the structures copied out are of different size so there might be
> a corner case for th getdents

Yes, this is what I suppose.

> 3) you seem to be confused.. getdents does NOT return strings it returns
> structures "somehow" fitted in a buffer

That doesn't matter. I give you the point of the fault (as I see it).

Hm, you say "somehow" fitted. The buffer may be a string without '\0'
at the end. Strncpy() may be used to concatenate those buffers. May be
something else...

> anyway... I looked at the sources and I found a strange thing so here's a
> patch...

> @@ -442,8 +443,7 @@

>         off = fp->f_offset;

> -       buflen = max(LINUX_DIRBLKSIZ, nbytes);
> -       buflen = min(buflen, MAXBSIZE);
> +       buflen = max(buflen, MAXBSIZE);
>         buf = malloc(buflen, M_TEMP, M_WAITOK);
>         vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, td);


> I am quite interested if this changes anything because the code is imho obviously wrong.

As for me I don't see any unusual here.

> it first limits buflen < 512

No. It limits buflen to which is greater, LINUX_DIRBLKSIZ or nbytes.

> and then to buflen = 64K.

No. It limits buflen to which is smaller, buflen or MAXBSIZE.

And your code here imho is equal to:
buflen = MAXBSIZE;

BTW, the system panics with this patch with a message "You have
allocated more memory than I have".

> I dont see how this could affect
> only 4096 bytes long buffers but its worth a try.

Instead of "only 4096" I'd say "at least 4096".


WBR
-- 
Boris Samorodov (bsam)
Research Engineer, http://www.ipt.ru Telephone & Internet SP
FreeBSD committer, http://www.FreeBSD.org The Power To Serve