| Message ID | 20230313220653.3996-1-todd.e.brandt@intel.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 7c3650a8007c43bc1a60d8bdb4cdbea3ad9a7dd4 |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | [v2] Fix buffer overrun in HID-SENSOR name string | expand |
Hi Todd, On Mon, Mar 13, 2023 at 03:06:53PM -0700, Todd Brandt wrote: > On some platforms there are some platform devices created with > invalid names. For example: "HID-SENSOR-INT-020b?.39.auto" instead > of "HID-SENSOR-INT-020b.39.auto" > > This string include some invalid characters, hence it will fail to > properly load the driver which will handle this custom sensor. Also > it is a problem for some user space tools, which parse the device > names from ftrace and dmesg. > > This is because the string, real_usage, is not NULL terminated and > printed with %s to form device name. > > To address this, we initialize the real_usage string with 0s. > > Philipp Jungkamp created this fix, I'm simply submitting it. I've > verified it fixes bugzilla issue 217169 > > Reported-and-tested-by: Todd Brandt <todd.e.brandt@linux.intel.com> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=217169 > Signed-off-by: Todd Brandt <todd.e.brandt@intel.com> Why is not Philip in the SoB list? Anyway the original patch made it to stable, so: Fixes: 98c062e82451 ("HID: hid-sensor-custom: Allow more custom iio sensors") Cc: stable@vger.kernel.org and with those you can add: Reviewed-by: Andi Shyti <andi.shyti@kernel.org> Andi
On Tue, 2023-03-14 at 00:07 +0100, Andi Shyti wrote: > Hi Todd, > > On Mon, Mar 13, 2023 at 03:06:53PM -0700, Todd Brandt wrote: > > On some platforms there are some platform devices created with > > invalid names. For example: "HID-SENSOR-INT-020b?.39.auto" instead > > of "HID-SENSOR-INT-020b.39.auto" > > > > This string include some invalid characters, hence it will fail to > > properly load the driver which will handle this custom sensor. Also > > it is a problem for some user space tools, which parse the device > > names from ftrace and dmesg. > > > > This is because the string, real_usage, is not NULL terminated and > > printed with %s to form device name. > > > > To address this, we initialize the real_usage string with 0s. > > > > Philipp Jungkamp created this fix, I'm simply submitting it. I've > > verified it fixes bugzilla issue 217169 > > > > Reported-and-tested-by: Todd Brandt <todd.e.brandt@linux.intel.com> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=217169 > > Signed-off-by: Todd Brandt <todd.e.brandt@intel.com> > > Why is not Philip in the SoB list? > Oh, sorry, I got feedback and assumed it needed more work on the changelog. I also forgot to copy over Phillip's SoB line so this one is probably worse than the original. The original was just fine, please ignore this one. > Anyway the original patch made it to stable, so: > > Fixes: 98c062e82451 ("HID: hid-sensor-custom: Allow more custom iio > sensors") > Cc: stable@vger.kernel.org > > and with those you can add: > > Reviewed-by: Andi Shyti <andi.shyti@kernel.org> > > Andi
On Mon, 2023-03-13 at 16:37 -0700, Todd Brandt wrote: > On Tue, 2023-03-14 at 00:07 +0100, Andi Shyti wrote: > > Hi Todd, > > > > On Mon, Mar 13, 2023 at 03:06:53PM -0700, Todd Brandt wrote: > > > On some platforms there are some platform devices created with > > > invalid names. For example: "HID-SENSOR-INT-020b?.39.auto" > > > instead > > > of "HID-SENSOR-INT-020b.39.auto" > > > > > > This string include some invalid characters, hence it will fail > > > to > > > properly load the driver which will handle this custom sensor. > > > Also > > > it is a problem for some user space tools, which parse s parse/parses > > > the device > > > names from ftrace and dmesg. > > > > > > This is because the string, real_usage, is not NULL terminated > > > and > > > printed with %s to form device name. > > > > > > To address this, we Remove "we" > > > initialize the real_usage string with 0s. > > > > > > Philipp Jungkamp created this fix, I'm simply submitting it. I've > > > verified it fixes bugzilla issue 217169 > > > You don't need the above two lines. You can add Original-by: Philipp Jungkamp <p.jungkamp@gmx.net> Before your SOB. > > > Reported-and-tested-by: Todd Brandt > > > <todd.e.brandt@linux.intel.com> > > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=217169 > > > Signed-off-by: Todd Brandt <todd.e.brandt@intel.com> > > > > Why is not Philip in the SoB list? > > > Oh, sorry, I got feedback and assumed it needed more work on the > changelog. I also forgot to copy over Phillip's SoB line so this one > is > probably worse than the original. The original was just fine, please > ignore this one. > > > Anyway the original patch made it to stable, so: > > > > Fixes: 98c062e82451 ("HID: hid-sensor-custom: Allow more custom iio > > sensors") > > Cc: stable@vger.kernel.org > > > > and with those you can add: > > > > Reviewed-by: Andi Shyti <andi.shyti@kernel.org> > > > > Andi >
On 13/03/2023 23:06, Todd Brandt wrote: > On some platforms there are some platform devices created with > invalid names. For example: "HID-SENSOR-INT-020b?.39.auto" instead > of "HID-SENSOR-INT-020b.39.auto" > > This string include some invalid characters, hence it will fail to > properly load the driver which will handle this custom sensor. Also > it is a problem for some user space tools, which parse the device > names from ftrace and dmesg. > > This is because the string, real_usage, is not NULL terminated and > printed with %s to form device name. > > To address this, we initialize the real_usage string with 0s. > > Philipp Jungkamp created this fix, I'm simply submitting it. I've > verified it fixes bugzilla issue 217169 > > Reported-and-tested-by: Todd Brandt <todd.e.brandt@linux.intel.com> > Signed-off-by: Todd Brandt <todd.e.brandt@intel.com> SoB denotes that you reported this and tested. Otherwise shall we start adding Reported and Tested tags to all of our commits? Best regards, Krzysztof
On Wed, 2023-03-15 at 08:04 +0100, Krzysztof Kozlowski wrote: > On 13/03/2023 23:06, Todd Brandt wrote: > > On some platforms there are some platform devices created with > > invalid names. For example: "HID-SENSOR-INT-020b?.39.auto" instead > > of "HID-SENSOR-INT-020b.39.auto" > > > > This string include some invalid characters, hence it will fail to > > properly load the driver which will handle this custom sensor. Also > > it is a problem for some user space tools, which parse the device > > names from ftrace and dmesg. > > > > This is because the string, real_usage, is not NULL terminated and > > printed with %s to form device name. > > > > To address this, we initialize the real_usage string with 0s. > > > > Philipp Jungkamp created this fix, I'm simply submitting it. I've > > verified it fixes bugzilla issue 217169 > > > > Reported-and-tested-by: Todd Brandt <todd.e.brandt@linux.intel.com> > > Signed-off-by: Todd Brandt <todd.e.brandt@intel.com> > > SoB denotes that you reported this and tested. Otherwise shall we > start > adding Reported and Tested tags to all of our commits? > I just copied it from someone else's post on the mailing list, I thought it was part of the standard lkml vernacular but I just looked it up and it's explicitly forbidden in Documentation/process/maintainer-top.rst line 385. So the answer is no. I'll not use it again. > > Best regards, > Krzysztof >
diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c index 3e3f89e01d81..d85398721659 100644 --- a/drivers/hid/hid-sensor-custom.c +++ b/drivers/hid/hid-sensor-custom.c @@ -940,7 +940,7 @@ hid_sensor_register_platform_device(struct platform_device *pdev, struct hid_sensor_hub_device *hsdev, const struct hid_sensor_custom_match *match) { - char real_usage[HID_SENSOR_USAGE_LENGTH]; + char real_usage[HID_SENSOR_USAGE_LENGTH] = { 0 }; struct platform_device *custom_pdev; const char *dev_name; char *c;
On some platforms there are some platform devices created with invalid names. For example: "HID-SENSOR-INT-020b?.39.auto" instead of "HID-SENSOR-INT-020b.39.auto" This string include some invalid characters, hence it will fail to properly load the driver which will handle this custom sensor. Also it is a problem for some user space tools, which parse the device names from ftrace and dmesg. This is because the string, real_usage, is not NULL terminated and printed with %s to form device name. To address this, we initialize the real_usage string with 0s. Philipp Jungkamp created this fix, I'm simply submitting it. I've verified it fixes bugzilla issue 217169 Reported-and-tested-by: Todd Brandt <todd.e.brandt@linux.intel.com> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217169 Signed-off-by: Todd Brandt <todd.e.brandt@intel.com> --- drivers/hid/hid-sensor-custom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)