mptfusion: prevent some memory corruption
Subsystems (3)
PATCH 1/1 mptfusion: prevent some memory corruption
These are signed values the come from the user, we put a cap on the upper bounds but not on the lower bounds.
We use "karg.dataSgeOffset" to calculate "sz". We verify "sz" and proceed as if that means that "karg.dataSgeOffset" is correct but this fails to consider that the "sz" calculations can have integer overflows.
---
During my QC process, I realized that I sent a similar patch last year, but never received a response.
http://permalink.gmane.org/gmane.linux.kernel.janitors/32590
Looking at both of them, I guess I prefer today's patch because it is simpler.
drivers/message/fusion/mptctl.c +9 -0
@@ -1859,6 +1859,15 @@ mptctl_do_mpt_command (struct mpt_ioctl_command karg, void __user *mfPtr) }spin_unlock_irqrestore (&ioc->taskmgmt_lock, flags); +/* Basic sanity checks to prevent underflows or integer overflows */ +if (karg.maxReplyBytes <0 || + karg.dataInSize <0 || + karg.dataOutSize <0 || + karg.dataSgeOffset <0 || + karg.maxSenseBytes <0 || + karg.dataSgeOffset > ioc->req_sz /4 ) +return -EINVAL ; +/* Verify that the final request frame will not be too large. */ sz = karg.dataSgeOffset *4 ;
Quoted 38 lines · Dan Carpenter
These are signed values the come from the user, we put a cap on the upper bounds but not on the lower bounds.
We use "karg.dataSgeOffset" to calculate "sz". We verify "sz" and proceed as if that means that "karg.dataSgeOffset" is correct but this fails to consider that the "sz" calculations can have integer overflows.
Signed-off-by Dan Carpenter <[email protected]>---
During my QC process, I realized that I sent a similar patch last year, but never received a response.http://permalink.gmane.org/gmane.linux.kernel.janitors/32590
Looking at both of them, I guess I prefer today's patch because it is simpler.
drivers/message/fusion/mptctl.c +9
-1@@ -1859,6 +1859,15 @@ mptctl_do_mpt_command (struct mpt_ioctl_command karg, void __user *mfPtr) }spin_unlock_irqrestore (&ioc->taskmgmt_lock, flags); +/* Basic sanity checks to prevent underflows or integer overflows */ +if (karg.maxReplyBytes <0 || + karg.dataInSize <0 || + karg.dataOutSize <0 || + karg.dataSgeOffset <0 || + karg.maxSenseBytes <0 || + karg.dataSgeOffset > ioc->req_sz /4 ) +return -EINVAL ; +/* Verify that the final request frame will not be too large. */ sz = karg.dataSgeOffset *4 ; --
Quoted 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 5 lines
Johannes Thumshirn Storage [email protected] +49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
Ping?
regards,
dan carpenter
Quoted 37 lines · Dan Carpenter
These are signed values the come from the user, we put a cap on the upper bounds but not on the lower bounds.
We use "karg.dataSgeOffset" to calculate "sz". We verify "sz" and proceed as if that means that "karg.dataSgeOffset" is correct but this fails to consider that the "sz" calculations can have integer overflows.
Signed-off-by Dan Carpenter <[email protected]>---
During my QC process, I realized that I sent a similar patch last year, but never received a response.http://permalink.gmane.org/gmane.linux.kernel.janitors/32590
Looking at both of them, I guess I prefer today's patch because it is simpler.
drivers/message/fusion/mptctl.c +9
-0@@ -1859,6 +1859,15 @@ mptctl_do_mpt_command (struct mpt_ioctl_command karg, void __user *mfPtr) }spin_unlock_irqrestore (&ioc->taskmgmt_lock, flags); +/* Basic sanity checks to prevent underflows or integer overflows */ +if (karg.maxReplyBytes <0 || + karg.dataInSize <0 || + karg.dataOutSize <0 || + karg.dataSgeOffset <0 || + karg.maxSenseBytes <0 || + karg.dataSgeOffset > ioc->req_sz /4 ) +return -EINVAL ; +/* Verify that the final request frame will not be too large. */ sz = karg.dataSgeOffset *4 ;