* [PATCH 7/18] netfilter: nfnetlink_log: Move away from NLMSG_PUT().
@ 2012-06-27 5:02 David Miller
2012-06-27 5:23 ` Joe Perches
2012-06-27 16:49 ` Ben Hutchings
0 siblings, 2 replies; 4+ messages in thread
From: David Miller @ 2012-06-27 5:02 UTC (permalink / raw)
To: netdev
And use nlmsg_data() while we're here too.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/netfilter/nfnetlink_log.c | 29 ++++++++++++++++-------------
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index 3c3cfc0..169ab59 100644
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -326,18 +326,20 @@ __nfulnl_send(struct nfulnl_instance *inst)
{
int status = -1;
- if (inst->qlen > 1)
- NLMSG_PUT(inst->skb, 0, 0,
- NLMSG_DONE,
- sizeof(struct nfgenmsg));
-
+ if (inst->qlen > 1) {
+ struct nlmsghdr *nlh = nlmsg_put(inst->skb, 0, 0,
+ NLMSG_DONE,
+ sizeof(struct nfgenmsg),
+ 0);
+ if (!nlh)
+ goto out;
+ }
status = nfnetlink_unicast(inst->skb, &init_net, inst->peer_pid,
MSG_DONTWAIT);
inst->qlen = 0;
inst->skb = NULL;
-
-nlmsg_failure:
+out:
return status;
}
@@ -380,10 +382,12 @@ __build_packet_message(struct nfulnl_instance *inst,
struct nfgenmsg *nfmsg;
sk_buff_data_t old_tail = inst->skb->tail;
- nlh = NLMSG_PUT(inst->skb, 0, 0,
+ nlh = nlmsg_put(inst->skb, 0, 0,
NFNL_SUBSYS_ULOG << 8 | NFULNL_MSG_PACKET,
- sizeof(struct nfgenmsg));
- nfmsg = NLMSG_DATA(nlh);
+ sizeof(struct nfgenmsg), 0);
+ if (!nlh)
+ return -1;
+ nfmsg = nlmsg_data(nlh);
nfmsg->nfgen_family = pf;
nfmsg->version = NFNETLINK_V0;
nfmsg->res_id = htons(inst->group_num);
@@ -526,7 +530,7 @@ __build_packet_message(struct nfulnl_instance *inst,
if (skb_tailroom(inst->skb) < nla_total_size(data_len)) {
printk(KERN_WARNING "nfnetlink_log: no tailroom!\n");
- goto nlmsg_failure;
+ return -1;
}
nla = (struct nlattr *)skb_put(inst->skb, nla_total_size(data_len));
@@ -540,7 +544,6 @@ __build_packet_message(struct nfulnl_instance *inst,
nlh->nlmsg_len = inst->skb->tail - old_tail;
return 0;
-nlmsg_failure:
nla_put_failure:
PRINTR(KERN_ERR "nfnetlink_log: error creating log nlmsg\n");
return -1;
@@ -745,7 +748,7 @@ nfulnl_recv_config(struct sock *ctnl, struct sk_buff *skb,
const struct nlmsghdr *nlh,
const struct nlattr * const nfula[])
{
- struct nfgenmsg *nfmsg = NLMSG_DATA(nlh);
+ struct nfgenmsg *nfmsg = nlmsg_data(nlh);
u_int16_t group_num = ntohs(nfmsg->res_id);
struct nfulnl_instance *inst;
struct nfulnl_msg_config_cmd *cmd = NULL;
--
1.7.10.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 7/18] netfilter: nfnetlink_log: Move away from NLMSG_PUT().
2012-06-27 5:02 [PATCH 7/18] netfilter: nfnetlink_log: Move away from NLMSG_PUT() David Miller
@ 2012-06-27 5:23 ` Joe Perches
2012-06-27 16:49 ` Ben Hutchings
1 sibling, 0 replies; 4+ messages in thread
From: Joe Perches @ 2012-06-27 5:23 UTC (permalink / raw)
To: David Miller; +Cc: netdev
On Tue, 2012-06-26 at 22:02 -0700, David Miller wrote:
> And use nlmsg_data() while we're here too.
[]
> diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
[]
> @@ -326,18 +326,20 @@ __nfulnl_send(struct nfulnl_instance *inst)
> {
> int status = -1;
>
> - if (inst->qlen > 1)
> - NLMSG_PUT(inst->skb, 0, 0,
> - NLMSG_DONE,
> - sizeof(struct nfgenmsg));
> -
> + if (inst->qlen > 1) {
> + struct nlmsghdr *nlh = nlmsg_put(inst->skb, 0, 0,
> + NLMSG_DONE,
> + sizeof(struct nfgenmsg),
> + 0);
> + if (!nlh)
> + goto out;
> + }
Because nlh isn't used for anything other than a test,
perhaps this is more readable as:
if (inst->qlen > 1 &&
!nlmsg_put(inst->skb, 0, 0, NLMSG_DONE,
sizeof(struct nfgenmsg), 0))
goto out;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 7/18] netfilter: nfnetlink_log: Move away from NLMSG_PUT().
2012-06-27 5:02 [PATCH 7/18] netfilter: nfnetlink_log: Move away from NLMSG_PUT() David Miller
2012-06-27 5:23 ` Joe Perches
@ 2012-06-27 16:49 ` Ben Hutchings
2012-06-27 22:06 ` David Miller
1 sibling, 1 reply; 4+ messages in thread
From: Ben Hutchings @ 2012-06-27 16:49 UTC (permalink / raw)
To: David Miller; +Cc: netdev
On Tue, 2012-06-26 at 22:02 -0700, David Miller wrote:
> And use nlmsg_data() while we're here too.
>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
> net/netfilter/nfnetlink_log.c | 29 ++++++++++++++++-------------
> 1 file changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
> index 3c3cfc0..169ab59 100644
> --- a/net/netfilter/nfnetlink_log.c
> +++ b/net/netfilter/nfnetlink_log.c
> @@ -326,18 +326,20 @@ __nfulnl_send(struct nfulnl_instance *inst)
> {
> int status = -1;
>
> - if (inst->qlen > 1)
> - NLMSG_PUT(inst->skb, 0, 0,
> - NLMSG_DONE,
> - sizeof(struct nfgenmsg));
> -
> + if (inst->qlen > 1) {
> + struct nlmsghdr *nlh = nlmsg_put(inst->skb, 0, 0,
> + NLMSG_DONE,
> + sizeof(struct nfgenmsg),
> + 0);
> + if (!nlh)
> + goto out;
> + }
> status = nfnetlink_unicast(inst->skb, &init_net, inst->peer_pid,
> MSG_DONTWAIT);
>
> inst->qlen = 0;
> inst->skb = NULL;
> -
> -nlmsg_failure:
> +out:
> return status;
> }
[...]
It looks like this also leaks the skb on failure. At least,
__nfulnl_flush(inst) is expected to dipose of inst->skb.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 7/18] netfilter: nfnetlink_log: Move away from NLMSG_PUT().
2012-06-27 16:49 ` Ben Hutchings
@ 2012-06-27 22:06 ` David Miller
0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2012-06-27 22:06 UTC (permalink / raw)
To: bhutchings; +Cc: netdev
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 27 Jun 2012 17:49:33 +0100
> It looks like this also leaks the skb on failure. At least,
> __nfulnl_flush(inst) is expected to dipose of inst->skb.
I did not change the behavior of this function, if someone wants
to fix this bug that's great, but I'm certainly not obligated to
do so.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-06-27 22:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-27 5:02 [PATCH 7/18] netfilter: nfnetlink_log: Move away from NLMSG_PUT() David Miller
2012-06-27 5:23 ` Joe Perches
2012-06-27 16:49 ` Ben Hutchings
2012-06-27 22:06 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.