On 28-10-2014 18:12, Pablo Neira Ayuso wrote:
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.
Cool, thanks. Marcelo -- 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
