| Message ID | 20230327094116.1763201-1-Igor.A.Artemiev@mcst.ru (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [lvc-project] netfilter: nfnetlink: NULL-check skb->dev in __build_packet_message() | expand |
On Mon, Mar 27, 2023 at 12:41:16PM +0300, Igor Artemiev wrote: > After having been compared to NULL value at nfnetlink_log.c:560, > pointer 'skb->dev' is dereferenced at nfnetlink_log.c:576. > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > Signed-off-by: Igor Artemiev <Igor.A.Artemiev@mcst.ru> > --- > net/netfilter/nfnetlink_log.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c > index d97eb280cb2e..2711509eb9a5 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)) { This cannot ever happen, we assume skb->dev is always set on. > 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))) > -- > 2.30.2 >
On 3/27/23 13:22, Pablo Neira Ayuso wrote: > On Mon, Mar 27, 2023 at 12:41:16PM +0300, Igor Artemiev wrote: >> After having been compared to NULL value at nfnetlink_log.c:560, >> pointer 'skb->dev' is dereferenced at nfnetlink_log.c:576. >> >> Found by Linux Verification Center (linuxtesting.org) with SVACE. >> >> Signed-off-by: Igor Artemiev <Igor.A.Artemiev@mcst.ru> >> --- >> net/netfilter/nfnetlink_log.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c >> index d97eb280cb2e..2711509eb9a5 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)) { > This cannot ever happen, we assume skb->dev is always set on. > >> 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))) >> -- >> 2.30.2 >> If skb->dev is always set on, should the check at nfnetlink_log.c:560 be removed? | if (indev && skb->dev && skb_mac_header_was_set(skb) && skb_mac_header_len(skb) != 0) { | ||Thanks, Igor
diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c index d97eb280cb2e..2711509eb9a5 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)))
After having been compared to NULL value at nfnetlink_log.c:560, pointer 'skb->dev' is dereferenced at nfnetlink_log.c:576. Found by Linux Verification Center (linuxtesting.org) with SVACE. Signed-off-by: Igor Artemiev <Igor.A.Artemiev@mcst.ru> --- net/netfilter/nfnetlink_log.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)