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.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
