| Message ID | 20221202083304.9005-1-liqiong@nfschina.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | netfilter: nfnetlink: check 'skb->dev' pointer in nfulnl_log_packet() | expand |
On Fri, Dec 02, 2022 at 04:33:04PM +0800, Li Qiong wrote: > The 'skb->dev' may be NULL, it should be better to check it. > > Signed-off-by: Li Qiong <liqiong@nfschina.com> > --- This needs a Fixes tag as well. Here my records show that I looked at this in 2018 but I probably ignored it based on that the code was introduced in 2012. When warnings are really ancient sometimes I just assume they must be false positives or someone would have hit it in testing. Also for really ancient bugs, it's hard to contact the original author or they have forgotten the details of the code. regards, dan carpenter
On Fri, Dec 02, 2022 at 04:33:04PM +0800, Li Qiong wrote: > The 'skb->dev' may be NULL, it should be better to check it. > > Signed-off-by: Li Qiong <liqiong@nfschina.com> > --- > net/netfilter/nfnetlink_log.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c > index d97eb280cb2e..74ac9fa40137 100644 > --- a/net/netfilter/nfnetlink_log.c > +++ b/net/netfilter/nfnetlink_log.c > @@ -572,7 +572,7 @@ __build_packet_message(struct nfnl_log_net *log, > } > } > > - if (indev && skb_mac_header_was_set(skb)) { > + if (indev && skb->dev && skb_mac_header_was_set(skb)) { > if (nla_put_be16(inst->skb, NFULA_HWTYPE, htons(skb->dev->type)) || > nla_put_be16(inst->skb, NFULA_HWLEN, > htons(skb->dev->hard_header_len))) > @@ -724,7 +724,7 @@ nfulnl_log_packet(struct net *net, > + nla_total_size(sizeof(struct nfulnl_msg_packet_timestamp)) > + nla_total_size(sizeof(struct nfgenmsg)); /* NLMSG_DONE */ > > - if (in && skb_mac_header_was_set(skb)) { > + if (in && skb->dev && skb_mac_header_was_set(skb)) { > size += nla_total_size(skb->dev->hard_header_len) > + nla_total_size(sizeof(u_int16_t)) /* hwtype */ > + nla_total_size(sizeof(u_int16_t)); /* hwlen */ skb->dev is always guaranteed to be set in this path.
diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c index d97eb280cb2e..74ac9fa40137 100644 --- a/net/netfilter/nfnetlink_log.c +++ b/net/netfilter/nfnetlink_log.c @@ -572,7 +572,7 @@ __build_packet_message(struct nfnl_log_net *log, } } - if (indev && skb_mac_header_was_set(skb)) { + if (indev && skb->dev && skb_mac_header_was_set(skb)) { if (nla_put_be16(inst->skb, NFULA_HWTYPE, htons(skb->dev->type)) || nla_put_be16(inst->skb, NFULA_HWLEN, htons(skb->dev->hard_header_len))) @@ -724,7 +724,7 @@ nfulnl_log_packet(struct net *net, + nla_total_size(sizeof(struct nfulnl_msg_packet_timestamp)) + nla_total_size(sizeof(struct nfgenmsg)); /* NLMSG_DONE */ - if (in && skb_mac_header_was_set(skb)) { + if (in && skb->dev && skb_mac_header_was_set(skb)) { size += nla_total_size(skb->dev->hard_header_len) + nla_total_size(sizeof(u_int16_t)) /* hwtype */ + nla_total_size(sizeof(u_int16_t)); /* hwlen */
The 'skb->dev' may be NULL, it should be better to check it. Signed-off-by: Li Qiong <liqiong@nfschina.com> --- net/netfilter/nfnetlink_log.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)