Hi Yuval,

Thanks for your review and all your valuable comments.

I take all of them into account, except this one, for which I
am at a loss for what to do :

> In build_permissions() you use:
> adminowns = !memcmp(usid,adminsid(),usidsz) || 
> !memcmp(gsid,adminsid(),gsidsz);
>
> You assume that usidsz >= 16. It is unlikely to crash if not, but it
> is still an assumption you should not make. (think fuzzing)
>   

Where is the assumption ? I have never heard of a limit
in memcmp(). On some architectures, when fetching a byte,
you actually fetch a few unneeded bytes possibly beyond
end of memory allocated to process, but that should be
taken into account by the linker (or malloc()).

Another point, about endianness : I have redefined macroes
to check endianness at compile-time, but I had to redefine
many fields in layout.h because most disk images are
defined as u16, u32 etc. instead of le16, le32, etc.

Has this been done on purpose ?

I think the option of enabling compile-time checks is
useful. I have found several errors in my code, but also
two errors outside my code :

in ntfs_ie_get_next() from index.c there is a le16_to_cpu()
which should be le32_to_cpu() and is wrong on BE boxes.

in ACCESS_ALLOWED_ACE from layout.h, the field
mask is defined as an enum whose size depends both on
the architecture and the compiler, whereas it should be
defined as a le32.

Regards

Jean-Pierre


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
ntfs-3g-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ntfs-3g-devel

Reply via email to