| Message ID | 20100603095613.GP5483@bicker |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
Dan Carpenter wrote: > copy_from_user() returns the number of bytes remaining but we should > return -EFAULT here. The error code gets returned to the user. Both > old_capi_manufacturer() and capi20_manufacturer() had other places > that already returned -EFAULT so this won't break anything. > Good point. > Signed-off-by: Dan Carpenter <error27@gmail.com> > > diff --git a/drivers/isdn/capi/kcapi.c b/drivers/isdn/capi/kcapi.c > index bde3c88..b054494 100644 > --- a/drivers/isdn/capi/kcapi.c > +++ b/drivers/isdn/capi/kcapi.c > @@ -1020,12 +1020,12 @@ static int old_capi_manufacturer(unsigned int cmd, void __user *data) > if (cmd == AVMB1_ADDCARD) { > if ((retval = copy_from_user(&cdef, data, > sizeof(avmb1_carddef)))) > - return retval; > + return -EFAULT; > cdef.cardtype = AVM_CARDTYPE_B1; > } else { > if ((retval = copy_from_user(&cdef, data, > sizeof(avmb1_extcarddef)))) > - return retval; > + return -EFAULT; > } > cparams.port = cdef.port; > cparams.irq = cdef.irq; > @@ -1218,7 +1218,7 @@ int capi20_manufacturer(unsigned int cmd, void __user *data) > kcapi_carddef cdef; > > if ((retval = copy_from_user(&cdef, data, sizeof(cdef)))) > - return retval; > + return -EFAULT; > > cparams.port = cdef.port; > cparams.irq = cdef.irq; No need to assign retval anymore, it is overwritten in all non-error cases. Jan
From: Jan Kiszka <jan.kiszka@web.de> Date: Thu, 03 Jun 2010 12:26:42 +0200 > No need to assign retval anymore, it is overwritten in all non-error cases. I'm still going to apply this fix as-is since it's easier to validate and provably won't introduce new compiler warnings. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Dan Carpenter <error27@gmail.com> Date: Thu, 3 Jun 2010 11:56:13 +0200 > copy_from_user() returns the number of bytes remaining but we should > return -EFAULT here. The error code gets returned to the user. Both > old_capi_manufacturer() and capi20_manufacturer() had other places > that already returned -EFAULT so this won't break anything. > > Signed-off-by: Dan Carpenter <error27@gmail.com> Applied. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/isdn/capi/kcapi.c b/drivers/isdn/capi/kcapi.c index bde3c88..b054494 100644 --- a/drivers/isdn/capi/kcapi.c +++ b/drivers/isdn/capi/kcapi.c @@ -1020,12 +1020,12 @@ static int old_capi_manufacturer(unsigned int cmd, void __user *data) if (cmd == AVMB1_ADDCARD) { if ((retval = copy_from_user(&cdef, data, sizeof(avmb1_carddef)))) - return retval; + return -EFAULT; cdef.cardtype = AVM_CARDTYPE_B1; } else { if ((retval = copy_from_user(&cdef, data, sizeof(avmb1_extcarddef)))) - return retval; + return -EFAULT; } cparams.port = cdef.port; cparams.irq = cdef.irq; @@ -1218,7 +1218,7 @@ int capi20_manufacturer(unsigned int cmd, void __user *data) kcapi_carddef cdef; if ((retval = copy_from_user(&cdef, data, sizeof(cdef)))) - return retval; + return -EFAULT; cparams.port = cdef.port; cparams.irq = cdef.irq;
copy_from_user() returns the number of bytes remaining but we should return -EFAULT here. The error code gets returned to the user. Both old_capi_manufacturer() and capi20_manufacturer() had other places that already returned -EFAULT so this won't break anything. Signed-off-by: Dan Carpenter <error27@gmail.com> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html