Re: [PATCH]QNX6 filesystem (RO) driver

[Al Viro <viro@ZenIV.linux.org.uk>]

On Fri, Feb 17, 2012 at 10:38:12PM +0100, Kai Bankett wrote:
> At least code was robust enough to deal with it ;)
> I guess ino was set to 0 anyways by compiler? At least it does not
> seem to have been sort of random.

Nope.  Uninitialized is uninitialized - in reality it's "whatever happens
to be in that word on stack", in theory it's "compiler has every right
to do _anything_ to reads from it".  Nasal daemon country...

> Otherwise testing would have shown strange effects - right?

Depends.

> http://a6.ontika.net/patches/0001-fs-initial-qnx6fs-addition.patch.gz

Still a problem with memcmp() use in qnx6_long_match() - suppose you've
got lf_size (on corrupt fs) set to 3Kb.  With 2Kb blocks and odd
->de_long_inode.  You are doing a lookup for 3Kb name.  What we get is
qnx6_longname() returning a pointer 2Kb into 4Kb page.  And qnx6_long_match()
proceeding to do memcmp() on 3Kb starting from that point.  Oops - literally.

What we need to do is either
	* silently return 0 on thislen > QNX6_LONG_NAME_MAX, or
	* return 0 and yell about corrupt /longfiles, or
	* have qnx6_lookup() check that len <= QNX6_LONG_NAME_MAX and
return ERR_PTR(-ENAMETOOLONG) if it isn't.  And lose the thislen check
completely.

I'd probably go for the last variant; it's consistent with what other
filesystems are doing and it would avoid mess if we ever try to do
r/w variant.  BTW, thislen check in qnx6_match() is also pointless -
we'd already checked that in caller.  Otherwise it would be wrong in
the same way as for long ones...