| Message ID | 1420999276-28225-1-git-send-email-cj@linux.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Sun, 2015-01-11 at 13:01 -0500, Christoph Jaeger wrote: > Due to a misplaced parenthesis, the expression > > (unlikely(offset) < 0), > > which expands to > > (__builtin_expect(!!(offset), 0) < 0), > > never evaluates to true. Therefore, when sending packets with > PF_PACKET/SOCK_DGRAM, packet_snd() does not abort as intended > if the creation of the layer 2 header fails. > > Spotted by Coverity - CID 1259975 ("Operands don't affect result"). > > Fixes: 9c7077622dd9 ("packet: make packet_snd fail on len smaller than l2 header") > Signed-off-by: Christoph Jaeger <cj@linux.com> > --- Nice catch ! Acked-by: Eric Dumazet <edumazet@google.com> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Jan 11, 2015 at 1:35 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > On Sun, 2015-01-11 at 13:01 -0500, Christoph Jaeger wrote: >> Due to a misplaced parenthesis, the expression >> >> (unlikely(offset) < 0), >> >> which expands to >> >> (__builtin_expect(!!(offset), 0) < 0), >> >> never evaluates to true. Therefore, when sending packets with >> PF_PACKET/SOCK_DGRAM, packet_snd() does not abort as intended >> if the creation of the layer 2 header fails. >> >> Spotted by Coverity - CID 1259975 ("Operands don't affect result"). >> >> Fixes: 9c7077622dd9 ("packet: make packet_snd fail on len smaller than l2 header") >> Signed-off-by: Christoph Jaeger <cj@linux.com> >> --- > > Nice catch ! > > Acked-by: Eric Dumazet <edumazet@google.com> > Indeed. I'm responsible for that typo. Thanks a lot for catching it! Acked-by: Willem de Bruijn <willemb@google.com> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, 2015-01-11 at 13:01 -0500, Christoph Jaeger wrote: > Due to a misplaced parenthesis, the expression > > (unlikely(offset) < 0), > > which expands to > > (__builtin_expect(!!(offset), 0) < 0), Here's another one: drivers/platform/goldfish/goldfish_pipe.c:285: if (unlikely(bufflen) == 0) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Jan 11, 2015 at 10:52:25AM -0800, Joe Perches wrote: > On Sun, 2015-01-11 at 13:01 -0500, Christoph Jaeger wrote: > > Due to a misplaced parenthesis, the expression > > > > (unlikely(offset) < 0), > > > > which expands to > > > > (__builtin_expect(!!(offset), 0) < 0), > > Here's another one: > > drivers/platform/goldfish/goldfish_pipe.c:285: if (unlikely(bufflen) == 0) Well, the conditional statement works as intended. Of course, the branch prediction doesn't. Coccinelle should be able to check for this kind of likely()/unlikely() usage, shouldn't it? ~Christoph -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/11/2015 07:01 PM, Christoph Jaeger wrote: > Due to a misplaced parenthesis, the expression > > (unlikely(offset) < 0), > > which expands to > > (__builtin_expect(!!(offset), 0) < 0), > > never evaluates to true. Therefore, when sending packets with > PF_PACKET/SOCK_DGRAM, packet_snd() does not abort as intended > if the creation of the layer 2 header fails. > > Spotted by Coverity - CID 1259975 ("Operands don't affect result"). > > Fixes: 9c7077622dd9 ("packet: make packet_snd fail on len smaller than l2 header") > Signed-off-by: Christoph Jaeger <cj@linux.com> Thanks, Christoph! Acked-by: Daniel Borkmann <dborkman@redhat.com> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Christoph Jaeger <cj@linux.com> Date: Sun, 11 Jan 2015 13:01:16 -0500 > Due to a misplaced parenthesis, the expression > > (unlikely(offset) < 0), > > which expands to > > (__builtin_expect(!!(offset), 0) < 0), > > never evaluates to true. Therefore, when sending packets with > PF_PACKET/SOCK_DGRAM, packet_snd() does not abort as intended > if the creation of the layer 2 header fails. > > Spotted by Coverity - CID 1259975 ("Operands don't affect result"). > > Fixes: 9c7077622dd9 ("packet: make packet_snd fail on len smaller than l2 header") > Signed-off-by: Christoph Jaeger <cj@linux.com> Applied, thank you. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 6880f34..9cfe2e1 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -2517,7 +2517,7 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len) err = -EINVAL; if (sock->type == SOCK_DGRAM) { offset = dev_hard_header(skb, dev, ntohs(proto), addr, NULL, len); - if (unlikely(offset) < 0) + if (unlikely(offset < 0)) goto out_free; } else { if (ll_header_truncated(dev, len))
Due to a misplaced parenthesis, the expression (unlikely(offset) < 0), which expands to (__builtin_expect(!!(offset), 0) < 0), never evaluates to true. Therefore, when sending packets with PF_PACKET/SOCK_DGRAM, packet_snd() does not abort as intended if the creation of the layer 2 header fails. Spotted by Coverity - CID 1259975 ("Operands don't affect result"). Fixes: 9c7077622dd9 ("packet: make packet_snd fail on len smaller than l2 header") Signed-off-by: Christoph Jaeger <cj@linux.com> --- net/packet/af_packet.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)