Message ID | 4fe84646-eef5-1a33-5451-11a7800c3c9d@posteo.de (mailing list archive) |
---|---|
State | Rejected |
Delegated to: | David Ahern |
Headers | show |
Series | Ensure check of nlmsg length is performed before actual access | expand |
Context | Check | Description |
---|---|---|
netdev/tree_selection | success | Not a local patch |
On 11/30/2022 2:09 PM, maxdev@posteo.de wrote: > During a brief code review we noticed that the length field expected > inside the payload of the message is accessed before it is ensured that > the payload is large enough to actually hold this field. > > The people mentioned in the commit message helped in the overall code > review. > > Kind regards, > Max Hi, Typically patches would be sent directly as plain text in the email content, and not as an attachment. As this is a fix, you would also typically determine what commit this fixes, and add a "Fixes:" trailer to indicate this. You should also have the subject include either "net" or "net-next" along with PATCH inside the []. As for the patch contents itself, Linux still follows C89 rules for declarations, and you should leave "int len" at the top of the scope, but assign it after the validation check as you do. Thanks, Jake
From 89216bacbc44d6719668132626ffd66862be6dfc Mon Sep 17 00:00:00 2001 From: Max Kunzelmann <maxdev@posteo.de> Date: Wed, 23 Mar 2022 20:42:58 +0100 Subject: [PATCH] Ensure check of nlmsg length is performed before actual access Reviewed-by: Benny Baumann <BenBE@geshi.org> Reviewed-by: Robert Geislinger <github@crpykng.de> --- lib/libnetlink.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/libnetlink.c b/lib/libnetlink.c index 9af06232..0fe78943 100644 --- a/lib/libnetlink.c +++ b/lib/libnetlink.c @@ -732,13 +732,13 @@ int rtnl_dump_request_n(struct rtnl_handle *rth, struct nlmsghdr *n) static int rtnl_dump_done(struct nlmsghdr *h, const struct rtnl_dump_filter_arg *a) { - int len = *(int *)NLMSG_DATA(h); - if (h->nlmsg_len < NLMSG_LENGTH(sizeof(int))) { fprintf(stderr, "DONE truncated\n"); return -1; } + int len = *(int *)NLMSG_DATA(h); + if (len < 0) { errno = -len; -- 2.38.1