Re: [PATCH] um: Fix some error handling in uml_vector_user_bpf()

[Dan Carpenter <dan.carpenter@oracle.com>]

On Fri, Jan 24, 2020 at 04:48:03PM +0000, Anton Ivanov wrote:
> 
> 
> On 24/01/2020 16:44, Dan Carpenter wrote:
> > On Fri, Jan 24, 2020 at 12:52:18PM +0000, Anton Ivanov wrote:
> > > 
> > > 
> > > On 24/01/2020 10:14, Dan Carpenter wrote:
> > > > 1) The uml_vector_user_bpf() returns pointers so it should return NULL
> > > >      instead of false.
> > > > 2) If the "bpf_prog" allocation failed, it would have eventually lead to
> > > >      a crash.  We can't succeed after the error happens so it should just
> > > >      return.
> > > > 
> > > > Fixes: 9807019a62dc ("um: Loadable BPF "Firmware" for vector drivers")
> > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > > ---
> > > >    arch/um/drivers/vector_user.c | 10 +++++-----
> > > >    1 file changed, 5 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/arch/um/drivers/vector_user.c b/arch/um/drivers/vector_user.c
> > > > index ddcd917be0af..88483f5b034c 100644
> > > > --- a/arch/um/drivers/vector_user.c
> > > > +++ b/arch/um/drivers/vector_user.c
> > > > @@ -732,13 +732,13 @@ void *uml_vector_user_bpf(char *filename)
> > > >    	if (stat(filename, &statbuf) < 0) {
> > > >    		printk(KERN_ERR "Error %d reading bpf file", -errno);
> > > > -		return false;
> > > > +		return NULL;
> > > 
> > > I will sort this one out, thanks for noticing.
> > > 
> > > >    	}
> > > >    	bpf_prog = uml_kmalloc(sizeof(struct sock_fprog), UM_GFP_KERNEL);
> > > > -	if (bpf_prog != NULL) {
> > > > -		bpf_prog->len = statbuf.st_size / sizeof(struct sock_filter);
> > > > -		bpf_prog->filter = NULL;
> > > > -	}
> > > > +	if (!pfg_prog)
> > > 
> > > ^^^^^ ?
> > 
> > If we don't return here it leads to a NULL dereference.
> 
> It says pfg_prog
> 
> I cannot find this identifier :)
> 

Oh wow...  That's very embarrassing.  My QC scripts do compile these
as part of the process.  But this wasn't a in of my allmodconfig
and when I do "make arch/um/drivers/vector_user.o", it just silently
returns without printing anything.  I didn't notice that it hadn't
built.

Even "make V=2 arch/um/drivers/vector_user.o" doesn't generate output.

I will resend the patch (on Monday though).

regards,
dan carpenter

_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um