Re: [PATCH v3 2/3] Do error handling if __build_packet_message fails

[Marcelo Ricardo Leitner <mleitner@redhat.com>]

On 29-10-2014 10:04, Marcelo Ricardo Leitner wrote:
> Currently, we don't check if __build_packet_message fails or not and
> that leaves it prone to sending incomplete messages to userspace.
>
> This commit fixes it by canceling the new message if there was any issue
> while building it and makes sure the skb is freed if it was the only
> messsage in it.
>
> Signed-off-by: Marcelo Ricardo Leitner <mleitner@redhat.com>
> ---
>   net/netfilter/nfnetlink_log.c | 21 +++++++++++++++------
>   1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
> index b1e3a05794169283ed50d1c0fb4f44d9e7753eeb..05184a3d860885c10c3e977fdc47fb86e3528afa 100644
> --- a/net/netfilter/nfnetlink_log.c
> +++ b/net/netfilter/nfnetlink_log.c
> @@ -568,10 +568,8 @@ __build_packet_message(struct nfnl_log_net *log,
>   		struct nlattr *nla;
>   		int size = nla_attr_size(data_len);
>
> -		if (skb_tailroom(inst->skb) < nla_total_size(data_len)) {
> -			printk(KERN_WARNING "nfnetlink_log: no tailroom!\n");
> -			return -1;
> -		}
> +		if (skb_tailroom(inst->skb) < nla_total_size(data_len))
> +			goto nla_put_failure;
>
>   		nla = (struct nlattr *)skb_put(inst->skb, nla_total_size(data_len));
>   		nla->nla_type = NFULA_PAYLOAD;
> @@ -586,6 +584,7 @@ __build_packet_message(struct nfnl_log_net *log,
>
>   nla_put_failure:
>   	PRINTR(KERN_ERR "nfnetlink_log: error creating log nlmsg\n");
> +	nlmsg_cancel(skb, nlh);
>   	return -1;
>   }
>
> @@ -708,8 +707,9 @@ nfulnl_log_packet(struct net *net,
>
>   	inst->qlen++;
>
> -	__build_packet_message(log, inst, skb, data_len, pf,
> -				hooknum, in, out, prefix, plen);
> +	if (__build_packet_message(log, inst, skb, data_len, pf,
> +				hooknum, in, out, prefix, plen))

bad alignment, sorry.. I'll fix this.

Marcelo