~vdupras/duskos-discuss

3 2

[PATCH] fix posix VM tests on Cygwin (realpath issue)

Details
Message ID
<106958e0-5a4a-386d-7fd5-45a3f53161ab@gmx.de>
DKIM signature
missing
Download raw message
Hello,

I spent this evening trying to get "make test" work on my two test
environments. One is cygwin (on 64-bit Windows 10), the other is a
32-bit Linux VM (running on same 64-bit Windows 10 host OS).

The cygwin case was the easier one: In
<https://git.sr.ht/~vdupras/duskos/tree/master/item/posix/vm.c#L812>,
you call realpath() without checking the return value. When realpath is
called on cygwin with a non-existing file, it returns 0 with errno
ENOENT and the output buffer stays empty. On Linux, it also returns
errno ENOENT but the output buffer is filled with the path as if the
file did exist.

The attached Patch will make the real path return "/NoSuchFile". In both
OSes, stat() will now not find the file and FCHILD will push zero (There
are probably cleaner ways to fix this :D).

The Linux 32-bit case I was not able to solve fully. When commenting
some tests, others fail in a different way, so I would assume some
corruption or overflow issues. The comment in
<https://git.sr.ht/~vdupras/duskos/tree/master/item/fs/tests/lib/all.fs#L5>
make me believe you also stumbled on that.

I found some kind of reproducer by adding some debug output to FCHILD VM
function:

When you call the command

p" /fox"

(in particular, the path string must consist of a slash and exactly
three letters), on my (working) cygwin environment FCHILD is called with
the current directory and a pointer to memory where is written a byte of
value 3 and the three letters fox, resulting in path "fs/fox".

However, calling the same on Linux 32-bit, I see the pointer to memory
point to the "f" of "fox", resulting in a path like "fs/oxxx56789".

The digits come from the previous command p" /123456789" and are
leftover in the buffer used for the path. The additional two "x" are
always copies of the last letter of the file name.


I assume you have a better idea where to look in vm/boot.fs and all the
filesystem code to pinpoint this corruption better, so I'll leave this
one to you :-)


Regards,


Michael
Details
Message ID
<5adbcf16-1278-4bf2-8841-954abefb7bc4@app.fastmail.com>
In-Reply-To
<106958e0-5a4a-386d-7fd5-45a3f53161ab@gmx.de> (view parent)
DKIM signature
missing
Download raw message
On Sat, Nov 19, 2022, at 6:52 PM, Michael Schierl wrote:
> Hello,
>
> I spent this evening trying to get "make test" work on my two test
> environments. One is cygwin (on 64-bit Windows 10), the other is a
> 32-bit Linux VM (running on same 64-bit Windows 10 host OS).
>
> The cygwin case was the easier one: In
> <https://git.sr.ht/~vdupras/duskos/tree/master/item/posix/vm.c#L812>,
> you call realpath() without checking the return value. When realpath is
> called on cygwin with a non-existing file, it returns 0 with errno
> ENOENT and the output buffer stays empty. On Linux, it also returns
> errno ENOENT but the output buffer is filled with the path as if the
> file did exist.
>
> The attached Patch will make the real path return "/NoSuchFile". In both
> OSes, stat() will now not find the file and FCHILD will push zero (There
> are probably cleaner ways to fix this :D).
>
> The Linux 32-bit case I was not able to solve fully. When commenting
> some tests, others fail in a different way, so I would assume some
> corruption or overflow issues. The comment in
> <https://git.sr.ht/~vdupras/duskos/tree/master/item/fs/tests/lib/all.fs#L5>
> make me believe you also stumbled on that.

I don't think it's the same problem. The problem I refer to at this link happens
only on i386 (native), not the POSIX VM.

>
> I found some kind of reproducer by adding some debug output to FCHILD VM
> function:
>
> When you call the command
>
> p" /fox"
>
> (in particular, the path string must consist of a slash and exactly
> three letters), on my (working) cygwin environment FCHILD is called with
> the current directory and a pointer to memory where is written a byte of
> value 3 and the three letters fox, resulting in path "fs/fox".
>
> However, calling the same on Linux 32-bit, I see the pointer to memory
> point to the "f" of "fox", resulting in a path like "fs/oxxx56789".
>
> The digits come from the previous command p" /123456789" and are
> leftover in the buffer used for the path. The additional two "x" are
> always copies of the last letter of the file name.
>
>
> I assume you have a better idea where to look in vm/boot.fs and all the
> filesystem code to pinpoint this corruption better, so I'll leave this
> one to you :-)
>
>
> Regards,
>
>
> Michael
>
> Attachments:
> * 0001-POSIX-VM-Make-invalid-paths-work-on-Cygwin.patch

I had never tried running Dusk's POSIX VM on a 32-bit Linux before. Now I did,
and indeed, there's some bug about paths preventing tests from working. Thanks
for the heads up, I'll look into it.

Virgil
Details
Message ID
<803b7811-1aeb-498e-bc0b-64e164abee86@app.fastmail.com>
In-Reply-To
<106958e0-5a4a-386d-7fd5-45a3f53161ab@gmx.de> (view parent)
DKIM signature
missing
Download raw message
Hello Michael,

It was memcpy()'s fault. By chance, my environment allowed memcpy() to copy
overlapping areas and yours (and my test 32-bit VM) didn't. Fix is in.

Wait... now that I'm thinking about it, "move" isn't supposed to allow
overlapping memory areas. i386 Dusk kernel has no special logic for this. My fix
will allow you to run the tests, but it's a bad fix. On the contrary, my POSIX
MOVE() should check for overlapping conditions and abort so that faulty moves
are easier to detect.

Virgil
Details
Message ID
<a59d17ed22c2f16502c07c891f0cf8d9@vaughan.pe>
In-Reply-To
<803b7811-1aeb-498e-bc0b-64e164abee86@app.fastmail.com> (view parent)
DKIM signature
missing
Download raw message
Hi Virgil,

On 2022-11-20 07:58, Virgil Dupras wrote:
> Wait... now that I'm thinking about it, "move" isn't supposed to allow
> overlapping memory areas. i386 Dusk kernel has no special logic for 
> this. My fix
> will allow you to run the tests, but it's a bad fix. On the contrary, 
> my POSIX
> MOVE() should check for overlapping conditions and abort so that faulty 
> moves
> are easier to detect.

Rather than aborting, the overlapping memory move algorithm is easy to 
implement from first principles...

  https://github.com/bigpotato920/algorithm/blob/master/memmove.c

Doesn't help with memcpy though, obviously.
Cheers,
Gary
Reply to thread Export thread (mbox)