Re: [PATCH v2 3/3] Make use of pr_fmt where applicable

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Tue, Oct 28, 2014 at 05:56:31PM -0200, Marcelo Ricardo Leitner wrote:
> Hi Pablo,
> 
> While we are at this, what about this one, does it really have to be a BUG() one?
> 
> __build_packet_message()
> ...
>     if (data_len) {
>         struct nlattr *nla;
>         int size = nla_attr_size(data_len);
> 
>         if (skb_tailroom(inst->skb) < nla_total_size(data_len))
>             goto nla_put_failure;           <-- already changed
> 
>         nla = (struct nlattr *)skb_put(inst->skb, nla_total_size(data_len));
>         nla->nla_type = NFULA_PAYLOAD;
>         nla->nla_len = size;
> 
>         if (skb_copy_bits(skb, 0, nla_data(nla), data_len))
>             BUG();         <--
>     }
> 
> Seems we could just put a goto nla_put_failure there too instead of
> bringing everything down. skb_copy_bits will only fail if we try to
> copy too much from the skb.. We could leave a WARN_ONCE in there if
> the idea is to catch nasty bugs in there. WDYT? I'm thinking in just
> sticking with a goto in there too.

I think this is most likely catching a malformed skbuff coming from
the driver, in that case we shouldn't go further. There's similar
handling in other part of the kernel.

This may also trigger the BUG() if data_len is miscalculated as you
said, but that seems less likely to happen to me.

I would leave that one as it is.