Re: [PATCH] fs: make d_path-like functions all have unsigned size

On Tue, Jul 27, 2021 at 02:50:19PM +0000, Al Viro wrote:

On Tue, Jul 27, 2021 at 12:36:25PM +0200, Greg Kroah-Hartman wrote:

quoted

When running static analysis tools to find where signed values could
potentially wrap the family of d_path() functions turn out to trigger a
lot of mess.  In evaluating the code, all of these usages seem safe, but
pointer math is involved so if a negative number is ever somehow passed
into these functions, memory can be traversed backwards in ways not
intended.

Resolve all of the abuguity by just making "size" an unsigned value,
which takes the guesswork out of everything involved.

TBH, I'm not sure it's the right approach.  Huge argument passed to d_path()
is a bad idea, no matter what.  Do you really want to have the damn thing
try and fill 3Gb of buffer, all while holding rcu_read_lock() and a global
spinlock or two?  Hell, s/3Gb/1Gb/ and it won't get any better...


How about we do this instead:

d_path(const struct path *path, char *buf, int buflen)
{
	if (unlikely((unsigned)buflen > 0x8000)) {
		buf += (unsigned)buflen - 0x8000;
		buflen = 0x8000;
	}
	as in mainline
}

and take care of both issues?

umm ... what if someone passes in -ENOMEM as buflen?  Not saying we
have such a path right now, but I could imagine it happening.

	if (unlikely(buflen < 0))
		return ERR_PTR(buflen);
	if (unlikely(buflen > 0x8000)) {
		buf += buflen - 0x8000;
		buflen = 0x8000;
	}
	...