I agree that the simpler fix is to change the type of 'adapno' to u32, which is the type of uioc.adapno to u32.
2013/12/14 Måns Rullgård <[email protected]>: > "Chen.Yu" <[email protected]> writes: > >> From: "Chen.Yu" <[email protected]> >> >> There is a potential integer overflow in megadev_ioctl() if >> userspace passes in a large u32 variable uioc.adapno. >> The int variable adapno would < 0, leading to an error >> array access for hdb_soft_state[adapno], or an error >> copy_to_user(uioc.uioc_uaddr, mcontroller+adapno,..) >> >> Reported-by: Wenliang Fan <[email protected]> >> Suggested-by: Qixue Xiao <[email protected]> >> Signed-off-by: Yu Chen <[email protected]> >> Reviewed-by: Levente Kurusa <[email protected]> >> --- >> drivers/scsi/megaraid.c | 15 ++++++++++++--- >> 1 file changed, 12 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c >> index 816db12..0b90c54 100644 >> --- a/drivers/scsi/megaraid.c >> +++ b/drivers/scsi/megaraid.c >> @@ -3099,7 +3099,10 @@ megadev_ioctl(struct file *filep, unsigned int cmd, >> unsigned long arg) >> /* >> * Which adapter >> */ >> - if( (adapno = GETADAP(uioc.adapno)) >= hba_count ) >> + adapno = GETADAP(uioc.adapno); >> + if( adapno < 0 ) >> + return (-EINVAL); >> + if( adapno >= hba_count ) >> return (-ENODEV); > > This relies on implementation-defined behaviour when converting an > unsigned integer to signed integer. A simpler and more robust fix is to > make the local variable 'adapno' unsigned. > > -- > Måns Rullgård > [email protected] -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
