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 <m...@mansr.com>: > "Chen.Yu" <chy...@gmail.com> writes: > >> From: "Chen.Yu" <chy...@gmail.com> >> >> 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 <fanwle...@gmail.com> >> Suggested-by: Qixue Xiao <xiaoqixu...@163.com> >> Signed-off-by: Yu Chen <chy...@gmail.com> >> Reviewed-by: Levente Kurusa <le...@linux.com> >> --- >> 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 > m...@mansr.com -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
