Messages in this thread | | | Date | Tue, 27 Jul 2021 12:19:55 +0100 | From | Matthew Wilcox <> | Subject | Re: [PATCH] fs: make d_path-like functions all have unsigned size |
| |
On Tue, Jul 27, 2021 at 12:36:25PM +0200, Greg Kroah-Hartman wrote: > 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.
> diff --git a/fs/d_path.c b/fs/d_path.c > index 23a53f7b5c71..7876b741a47e 100644 > --- a/fs/d_path.c > +++ b/fs/d_path.c > @@ -182,7 +182,7 @@ static int prepend_path(const struct path *path, > */ > char *__d_path(const struct path *path, > const struct path *root, > - char *buf, int buflen) > + char *buf, unsigned int buflen) > { > DECLARE_BUFFER(b, buf, buflen);
I have questions about the quality of the analysis tool you're using.
struct prepend_buffer { char *buf; int len; }; #define DECLARE_BUFFER(__name, __buf, __len) \ struct prepend_buffer __name = {.buf = __buf + __len, .len = __len}
Why is it not flagging the assignment of an unsigned int buflen to a signed int len?
> +char *__d_path(const struct path *, const struct path *, char *, unsigned int); > +char *d_absolute_path(const struct path *, char *, unsigned int); > +char *d_path(const struct path *, char *, unsigned int); > +char *dentry_path_raw(const struct dentry *, char *, unsigned int); > +char *dentry_path(const struct dentry *, char *, unsigned int);
While you're touching these declarations, please name the 'unsigned int' parameter. I don't care about the others; they are obvious, but an unsigned int might be flags, a length, or a small grey walrus.
| |