Re: general protection fault in devlink_info_serial_number_put
From: Vincent Mailhol <hidden>
Date: 2025-02-04 14:18:54
Also in:
linux-can, lkml
Subsystem:
networking [general], the rest · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
+To: Jiri Pirko +To: Jakub Kicinski +CC: David S. Miller +CC: Eric Dumazet +CC: Paolo Abeni +CC: Simon Horman +CC: netdev@vger.kernel.org On 04/02/2025 at 16:44, YAN KANG wrote:
Dear developers and maintainers, I found a new kernel NULL-Pointer-Dereference bug titiled "general protection fault in devlink_info_serial_number_put" while using modified syzkaller fuzzing tool. I Itested it on the latest Linux upstream version (6.13.0-rc7)related to ETAS ES58X CAN/USB DRIVER, and it was able to be triggered. The bug info is: kernel revision: v6.13-rc7 OOPS message: general protection fault in devlink_info_serial_number_put reproducer:YES After preliminary analysis, The root casue may be : in the function: es58x_devlink_info_get drivers/net/can/usb/etas_es58x/es58x_devlink.c es58x_dev->udev->serial == NULL ,but no check for it. devlink_info_serial_number_put(req, es58x_dev->udev->serial) triggers NPD . Fix suggestion: Check es58x_dev->udev->serial before deference pointer.
Thanks for the report. I acknowledge the issue: the serial number of a USB device may be NULL and I forget to check this condition. @netdev and devlink maintainers I can of course fix this locally, but this that this kind of issue looks like some nasty pitfall to me. So, I was wondering if it wouldn't be safer to add the NULL check in the framework instead of in the device. The netlink is not part of the hot path, so a NULL check should not have performance impacts. I am thinking of:
diff --git a/include/net/netlink.h b/include/net/netlink.h
index e015ffbed819..eaee9a1aa91f 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h@@ -1617,6 +1617,8 @@ static inline int nla_put_sint(struct sk_buff*skb, int attrtype, s64 value)
static inline int nla_put_string(struct sk_buff *skb, int attrtype,
const char *str)
{
+ if (!str)
+ return 0;
return nla_put(skb, attrtype, strlen(str) + 1, str);
}
Of course, it is also possible to do the check in
devlink_info_serial_number_put().
What do you think?
If you fix this issue, please add the following tag to the commit: Reported-by: yan kang <redacted> Reported-by: yue sun <samsun1006219@gmail.com I hope it helps. Best regards yan kang
Yours sincerely, Vincent Mailhol