On Tue, Oct 06, 2015 at 11:49:41PM +0200, Javier Martinez Canillas wrote:
> Hello Stephen,
>
> On 10/06/2015 11:01 PM, Stephen Kitt wrote:
> > On Tue, 6 Oct 2015 22:57:26 +0200, Stephen Kitt <steve@xxxxxxx> wrote:
> >> On Tue, 6 Oct 2015 21:51:55 +0300, Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> >> wrote:
> >>> The patch 999b874f4aa3: "Input: joydev - validate axis/button maps
> >>> before clobbering current ones" from Aug 25, 2009, leads to the
> >>> following static checker warning:
> >>>
> >>> drivers/input/joydev.c:466 joydev_handle_JSIOCSAXMAP()
> >>> error: 'abspam' dereferencing possible ERR_PTR()
> >>>
> >>> drivers/input/joydev.c
> >>> 437 static int joydev_handle_JSIOCSAXMAP(struct joydev *joydev,
> >>> 438 void __user *argp, size_t
> >>> len) 439 {
> >>> 440 __u8 *abspam;
> >>> 441 int i;
> >>> 442 int retval = 0;
> >>> 443
> >>> 444 len = min(len, sizeof(joydev->abspam));
> >>> 445
> >>> 446 /* Validate the map. */
> >>> 447 abspam = memdup_user(argp, len);
> >>> 448 if (IS_ERR(abspam)) {
> >>> 449 retval = PTR_ERR(abspam);
> >>> 450 goto out;
> >>>
> >>> out labels are error prone. It's safer to return directly.
> >>>
> >>> https://plus.google.com/106378716002406849458/posts/dnanfhQ4mHQ
> >>>
> >>> joydev_handle_JSIOCSBTNMAP() has the same issue.
> >>
> >> Perhaps I'm missing something here, but that's not the code I wrote, nor is
> >> it the code that's currently in the kernel. What I have in my copy of the
> >> kernel tree is
> >>
> >> /* Validate the map. */
> >> abspam = kmalloc(len, GFP_KERNEL);
> >> if (!abspam)
> >> return -ENOMEM;
> >>
> >> which does as you recommend. If you look up the commit you're referring to
> >> you'll see that's also the code as I wrote it back in 2009; I'm not sure
> >> where your IS_ERR() and PTR_ERR() stuff is coming from.
> >
> > After further investigation I'm guessing this is
> > https://lkml.org/lkml/2015/10/2/370, so cc'ing Javier and Dmitry.
> >
>
> It is indeed a bug introduced by my "cleanup" patch, sorry for the mess :(
>
> I double checked when posting the patch but got confused and used the old
> error logic. Following is a fixup patch [0].
>
> I don't know if Dmitry prefers to squash with the other patch since it
> didn't hit mainline yet or if not I can post it as a proper patch so he
> can pick it on his next branch.
The original patch is buried under a merge so I'll just apply this one
without squashing.
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
