| Message ID | 3203eb44-6e69-4bda-b585-426408cb75ee@web.de (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | HID: bpf: One function call less in call_hid_bpf_rdesc_fixup() after error detection | expand |
Hi, On 12/27/2023 2:24 AM, Markus Elfring wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Tue, 26 Dec 2023 19:13:25 +0100 > > The kfree() function was called in one case by the > call_hid_bpf_rdesc_fixup() function during error handling > even if the passed data structure member contained a null pointer. > This issue was detected by using the Coccinelle software. It is totally OK to free a null pointer through kfree() and the ENOMEM case is an unlikely case, so I don't think the patch is necessary. > > Thus adjust jump targets. > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> > --- > drivers/hid/bpf/hid_bpf_dispatch.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c > index d9ef45fcaeab..c84fe55be5ed 100644 > --- a/drivers/hid/bpf/hid_bpf_dispatch.c > +++ b/drivers/hid/bpf/hid_bpf_dispatch.c > @@ -118,17 +118,17 @@ u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *s > > ctx_kern.data = kzalloc(ctx_kern.ctx.allocated_size, GFP_KERNEL); > if (!ctx_kern.data) > - goto ignore_bpf; > + goto dup_mem; > > memcpy(ctx_kern.data, rdesc, min_t(unsigned int, *size, HID_MAX_DESCRIPTOR_SIZE)); > > ret = hid_bpf_prog_run(hdev, HID_BPF_PROG_TYPE_RDESC_FIXUP, &ctx_kern); > if (ret < 0) > - goto ignore_bpf; > + goto free_data; > > if (ret) { > if (ret > ctx_kern.ctx.allocated_size) > - goto ignore_bpf; > + goto free_data; > > *size = ret; > } > @@ -137,8 +137,9 @@ u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *s > > return rdesc; > > - ignore_bpf: > +free_data: > kfree(ctx_kern.data); > +dup_mem: > return kmemdup(rdesc, *size, GFP_KERNEL); > } > EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup); > -- > 2.43.0 > > > .
>> The kfree() function was called in one case by the >> call_hid_bpf_rdesc_fixup() function during error handling >> even if the passed data structure member contained a null pointer. >> This issue was detected by using the Coccinelle software. > > It is totally OK to free a null pointer through kfree() and the ENOMEM > case is an unlikely case, so I don't think the patch is necessary. Would you ever like to avoid redundant data processing a bit more? Regards, Markus
On Wed, Dec 27, 2023 at 09:19:27AM +0100, Markus Elfring wrote: > >> The kfree() function was called in one case by the > >> call_hid_bpf_rdesc_fixup() function during error handling > >> even if the passed data structure member contained a null pointer. > >> This issue was detected by using the Coccinelle software. > > > > It is totally OK to free a null pointer through kfree() and the ENOMEM > > case is an unlikely case, so I don't think the patch is necessary. > > Would you ever like to avoid redundant data processing a bit more? Hi, This is the semi-friendly patch-bot of Greg Kroah-Hartman. Markus, you seem to have sent a nonsensical or otherwise pointless review comment to a patch submission on a Linux kernel developer mailing list. I strongly suggest that you not do this anymore. Please do not bother developers who are actively working to produce patches and features with comments that, in the end, are a waste of time. Patch submitter, please ignore Markus's suggestion; you do not need to follow it at all. The person/bot/AI that sent it is being ignored by almost all Linux kernel maintainers for having a persistent pattern of behavior of producing distracting and pointless commentary, and inability to adapt to feedback. Please feel free to also ignore emails from them. thanks, greg k-h's patch email bot
diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c index d9ef45fcaeab..c84fe55be5ed 100644 --- a/drivers/hid/bpf/hid_bpf_dispatch.c +++ b/drivers/hid/bpf/hid_bpf_dispatch.c @@ -118,17 +118,17 @@ u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *s ctx_kern.data = kzalloc(ctx_kern.ctx.allocated_size, GFP_KERNEL); if (!ctx_kern.data) - goto ignore_bpf; + goto dup_mem; memcpy(ctx_kern.data, rdesc, min_t(unsigned int, *size, HID_MAX_DESCRIPTOR_SIZE)); ret = hid_bpf_prog_run(hdev, HID_BPF_PROG_TYPE_RDESC_FIXUP, &ctx_kern); if (ret < 0) - goto ignore_bpf; + goto free_data; if (ret) { if (ret > ctx_kern.ctx.allocated_size) - goto ignore_bpf; + goto free_data; *size = ret; } @@ -137,8 +137,9 @@ u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *s return rdesc; - ignore_bpf: +free_data: kfree(ctx_kern.data); +dup_mem: return kmemdup(rdesc, *size, GFP_KERNEL); } EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup);