On Fri, Feb 01, 2019 at 07:31:48AM +1300, Kees Cook wrote: > On Thu, Jan 31, 2019 at 11:51 PM Greg KH <[email protected]> wrote: > > Can you test the patch below? > > > > thanks, > > > > greg k-h > > > > -------------- > > > > diff --git a/kernel/relay.c b/kernel/relay.c > > index 04f248644e06..9e0f52375487 100644 > > --- a/kernel/relay.c > > +++ b/kernel/relay.c > > @@ -428,6 +428,8 @@ static struct dentry *relay_create_buf_file(struct > > rchan *chan, > > dentry = chan->cb->create_buf_file(tmpname, chan->parent, > > S_IRUSR, buf, > > &chan->is_global); > > + if (IS_ERR(dentry)) > > + dentry = NULL; > > > > kfree(tmpname); > > > > @@ -461,7 +463,7 @@ static struct rchan_buf *relay_open_buf(struct rchan > > *chan, unsigned int cpu) > > dentry = chan->cb->create_buf_file(NULL, NULL, > > S_IRUSR, buf, > > &chan->is_global); > > - if (WARN_ON(dentry)) > > + if (IS_ERR_OR_NULL(dentry)) > > goto free_buf; > > } > > > > Thanks! (Can we find other cases of this with static analysis?)
Probably. I have over 100 patches to help clean up a lot of the debugfs mess. But it is very rare that someone actually tries to use the result of a debugfs call as a "real" dentry, except to pass it back into another debugfs call. I "think" I have now caught all of those cases, and if you can come up with some kind of rule for this, that would be great. But note, the create_buf_file() callback is the one that does the debugfs call, so trying to figure out where that is coming from, what it does, and what the dentry is later used for, spans lots of different subsystems and files. I don't think we have tools to do that, other than grep :) greg k-h
