| Message ID | 20211217071209.GF26548@kili (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | platform/x86: think-lmi: Prevent underflow in index_store() | expand |
Thanks Dan On 2021-12-17 02:12, Dan Carpenter wrote: > There needs to be a check to prevent negative offsets for > setting->index. I have reviewed this code and I think that the > "if (block->instance_count <= instance)" check in __query_block() will > prevent this from resulting in an out of bounds access. But it's > still worth fixing. > > Fixes: 640a5fa50a42 ("platform/x86: think-lmi: Opcode support") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > drivers/platform/x86/think-lmi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c > index 27ab8e4e5b83..0b73e16cccea 100644 > --- a/drivers/platform/x86/think-lmi.c > +++ b/drivers/platform/x86/think-lmi.c > @@ -573,7 +573,7 @@ static ssize_t index_store(struct kobject *kobj, > if (err < 0) > return err; > > - if (val > TLMI_INDEX_MAX) > + if (val < 0 || val > TLMI_INDEX_MAX) > return -EINVAL; > > setting->index = val; > Agreed, it's good to have this check. Thank you Mark
Hi, On 12/17/21 08:12, Dan Carpenter wrote: > There needs to be a check to prevent negative offsets for > setting->index. I have reviewed this code and I think that the > "if (block->instance_count <= instance)" check in __query_block() will > prevent this from resulting in an out of bounds access. But it's > still worth fixing. > > Fixes: 640a5fa50a42 ("platform/x86: think-lmi: Opcode support") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Thank you for your patch, I've applied this patch to my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans Note it will show up in my review-hans branch once I've pushed my local branch there, which might take a while. Once I've run some tests on this branch the patches there will be added to the platform-drivers-x86/for-next branch and eventually will be included in the pdx86 pull-request to Linus for the next merge-window. Regards, Hans > --- > drivers/platform/x86/think-lmi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c > index 27ab8e4e5b83..0b73e16cccea 100644 > --- a/drivers/platform/x86/think-lmi.c > +++ b/drivers/platform/x86/think-lmi.c > @@ -573,7 +573,7 @@ static ssize_t index_store(struct kobject *kobj, > if (err < 0) > return err; > > - if (val > TLMI_INDEX_MAX) > + if (val < 0 || val > TLMI_INDEX_MAX) > return -EINVAL; > > setting->index = val; >
diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c index 27ab8e4e5b83..0b73e16cccea 100644 --- a/drivers/platform/x86/think-lmi.c +++ b/drivers/platform/x86/think-lmi.c @@ -573,7 +573,7 @@ static ssize_t index_store(struct kobject *kobj, if (err < 0) return err; - if (val > TLMI_INDEX_MAX) + if (val < 0 || val > TLMI_INDEX_MAX) return -EINVAL; setting->index = val;
There needs to be a check to prevent negative offsets for setting->index. I have reviewed this code and I think that the "if (block->instance_count <= instance)" check in __query_block() will prevent this from resulting in an out of bounds access. But it's still worth fixing. Fixes: 640a5fa50a42 ("platform/x86: think-lmi: Opcode support") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- drivers/platform/x86/think-lmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)