| Message ID | 20210403151312.31796-1-paskripkin@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | net: netlink: fix error check in genl_family_rcv_msg_doit | expand |
Hi! On Sat, 2021-04-03 at 18:26 +0200, Johannes Berg wrote: > On Sat, 2021-04-03 at 15:13 +0000, Pavel Skripkin wrote: > > genl_family_rcv_msg_attrs_parse() can return NULL > > pointer: > > > > if (!ops->maxattr) > > return NULL; > > > > But this condition doesn't cause an error in > > genl_family_rcv_msg_doit > > And I'm almost certain that in fact it shouldn't cause an error! > > If the family doesn't set maxattr then it doesn't want to have > generic > netlink doing the parsing, but still it should be possible to call > the > ops. Look at fs/dlm/netlink.c for example, it doesn't even have > attributes. You're breaking it with this patch. > > Also, the (NULL) pointer is not actually _used_ anywhere, so why > would > it matter? > Oh, I see now. I thought, it could cause a NULL ptr deference in some cases, because some ->doit() functions accessing info.attrs directly. Now I understand the point, sorry for my misunderstanding the situation. > johannes > With regards, Pavel Skripkin
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c index 2d6fdf40df66..c06d06ead181 100644 --- a/net/netlink/genetlink.c +++ b/net/netlink/genetlink.c @@ -719,6 +719,8 @@ static int genl_family_rcv_msg_doit(const struct genl_family *family, GENL_DONT_VALIDATE_STRICT); if (IS_ERR(attrbuf)) return PTR_ERR(attrbuf); + if (!attrbuf) + return -EINVAL; info.snd_seq = nlh->nlmsg_seq; info.snd_portid = NETLINK_CB(skb).portid;
genl_family_rcv_msg_attrs_parse() can return NULL pointer: if (!ops->maxattr) return NULL; But this condition doesn't cause an error in genl_family_rcv_msg_doit Signed-off-by: Pavel Skripkin <paskripkin@gmail.com> --- net/netlink/genetlink.c | 2 ++ 1 file changed, 2 insertions(+)