Re: [PATCH 02/13] IP set core support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 1 Feb 2011, Jozsef Kadlecsik wrote:

> On Tue, 1 Feb 2011, Patrick McHardy wrote:
> 
> > Am 31.01.2011 23:52, schrieb Jozsef Kadlecsik:
> > > +static int
> > > +call_ad(struct sk_buff *skb, struct ip_set *set,
> > > +	struct nlattr *tb[], enum ipset_adt adt,
> > > +	u32 flags, bool use_lineno)
> > > +{
> > > +	int ret, retried = 0;
> > > +	u32 lineno = 0;
> > > +	bool eexist = flags & IPSET_FLAG_EXIST;
> > > +
> > > +	do {
> > > +		write_lock_bh(&set->lock);
> > > +		ret = set->variant->uadt(set, tb, adt, &lineno, flags);
> > > +		write_unlock_bh(&set->lock);
> > > +	} while (ret == -EAGAIN &&
> > > +		 set->variant->resize &&
> > > +		 (ret = set->variant->resize(set, retried++)) == 0);
> > > +
> > > +	if (!ret || (ret == -IPSET_ERR_EXIST && eexist))
> > > +		return 0;
> > > +	if (lineno && use_lineno) {
> > > +		/* Error in restore/batch mode: send back lineno */
> > > +		struct nlmsghdr *nlh = nlmsg_hdr(skb);
> > > +		int min_len = NLMSG_SPACE(sizeof(struct nfgenmsg));
> > > +		struct nlattr *cda[IPSET_ATTR_CMD_MAX+1];
> > > +		struct nlattr *cmdattr = (void *)nlh + min_len;
> > > +		u32 *errline;
> > > +
> > > +		nla_parse(cda, IPSET_ATTR_CMD_MAX,
> > > +			  cmdattr, nlh->nlmsg_len - min_len,
> > > +			  ip_set_adt_policy);
> > > +
> > > +		errline = nla_data(cda[IPSET_ATTR_LINENO]);
> > > +
> > > +		*errline = lineno;
> > 
> > This is still not correct. I didn't mean to remove the const attributes
> > (the message is still considered const by the higher layers, the netlink
> > functions just cast this away). You're modifying the received message,
> > I don't see how this can be useful to userspace.
> 
> I can't find where the message is considered const in netlink/nfnetlink.
> It seems to be freely writable via skb.
>  
> > I guess you're relying on that the original message is appended to a
> > nlmsgerr message. That doesn't seem right though, if you want to return
> > something to userspace, you should construct a new message.
> 
> The message we are processing here carried multiple commands (each having 
> an attribute with the line number of the given command) and one failed 
> from some reason. We have to notify the userspace which command, at what 
> line failed. For this reason the multi-command messages have got an 
> attribute, which can be filled out with the line number - that happens 
> here. The attribute is already there, the message is not enlarged, just
> the empty value is overwritten with the proper value.
> 
> The line number reporting works this way, tested in the testsuite too.
> 
> If I had to construct a completely new message and sent it, that'd be more 
> or less the duplication of netlink_ack. Additionally I had to suppress 
> netlink from sending an errmsg/ack too.

Hm, if I lie -EINTR to netlink, then I can construct and send the error 
message manually and keep NLM_F_ACK at the same time. What do you think?
Please have a look at the attached patch.

Best regards,
Jozsef
-
E-mail  : kadlec@xxxxxxxxxxxxxxxxx, kadlec@xxxxxxxxxxxx
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary
diff --git a/kernel/ip_set_core.c b/kernel/ip_set_core.c
index 19158bf..cc67d92 100644
--- a/kernel/ip_set_core.c
+++ b/kernel/ip_set_core.c
@@ -1103,7 +1103,7 @@ static const struct nla_policy ip_set_adt_policy[IPSET_ATTR_CMD_MAX + 1] = {
 };
 
 static int
-call_ad(struct sk_buff *skb, struct ip_set *set,
+call_ad(struct sock *ctnl, struct sk_buff *skb, struct ip_set *set,
 	struct nlattr *tb[], enum ipset_adt adt,
 	u32 flags, bool use_lineno)
 {
@@ -1123,12 +1123,25 @@ call_ad(struct sk_buff *skb, struct ip_set *set,
 		return 0;
 	if (lineno && use_lineno) {
 		/* Error in restore/batch mode: send back lineno */
-		struct nlmsghdr *nlh = nlmsg_hdr(skb);
+		struct nlmsghdr *rep, *nlh = nlmsg_hdr(skb);
+		struct sk_buff *skb2;
+		struct nlmsgerr *errmsg;
+		size_t payload = sizeof(*errmsg) + nlmsg_len(nlh);
 		int min_len = NLMSG_SPACE(sizeof(struct nfgenmsg));
 		struct nlattr *cda[IPSET_ATTR_CMD_MAX+1];
-		struct nlattr *cmdattr = (void *)nlh + min_len;
+		struct nlattr *cmdattr;
 		u32 *errline;
 
+		skb2 = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+		if (skb2 == NULL)
+			return -ENOMEM;
+		rep = __nlmsg_put(skb2, NETLINK_CB(skb).pid,
+				  nlh->nlmsg_seq, NLMSG_ERROR, payload, 0);
+		errmsg = nlmsg_data(rep);
+		errmsg->error = ret;
+		memcpy(&errmsg->msg, nlh, nlh->nlmsg_len);
+		cmdattr = (void *)&errmsg->msg + min_len;
+
 		nla_parse(cda, IPSET_ATTR_CMD_MAX,
 			  cmdattr, nlh->nlmsg_len - min_len,
 			  ip_set_adt_policy);
@@ -1136,6 +1149,9 @@ call_ad(struct sk_buff *skb, struct ip_set *set,
 		errline = nla_data(cda[IPSET_ATTR_LINENO]);
 
 		*errline = lineno;
+
+		netlink_unicast(ctnl, skb2, NETLINK_CB(skb).pid, MSG_DONTWAIT);
+		return -EINTR;
 	}
 
 	return ret;
@@ -1174,7 +1190,8 @@ ip_set_uadd(struct sock *ctnl, struct sk_buff *skb,
 				     attr[IPSET_ATTR_DATA],
 				     set->type->adt_policy))
 			return -IPSET_ERR_PROTOCOL;
-		ret = call_ad(skb, set, tb, IPSET_ADD, flags, use_lineno);
+		ret = call_ad(ctnl, skb, set, tb, IPSET_ADD, flags,
+			      use_lineno);
 	} else {
 		int nla_rem;
 
@@ -1185,7 +1202,7 @@ ip_set_uadd(struct sock *ctnl, struct sk_buff *skb,
 			    nla_parse_nested(tb, IPSET_ATTR_ADT_MAX, nla,
 					     set->type->adt_policy))
 				return -IPSET_ERR_PROTOCOL;
-			ret = call_ad(skb, set, tb, IPSET_ADD,
+			ret = call_ad(ctnl, skb, set, tb, IPSET_ADD,
 				      flags, use_lineno);
 			if (ret < 0)
 				return ret;
@@ -1227,7 +1244,8 @@ ip_set_udel(struct sock *ctnl, struct sk_buff *skb,
 				     attr[IPSET_ATTR_DATA],
 				     set->type->adt_policy))
 			return -IPSET_ERR_PROTOCOL;
-		ret = call_ad(skb, set, tb, IPSET_DEL, flags, use_lineno);
+		ret = call_ad(ctnl, skb, set, tb, IPSET_DEL, flags,
+			      use_lineno);
 	} else {
 		int nla_rem;
 
@@ -1238,7 +1256,7 @@ ip_set_udel(struct sock *ctnl, struct sk_buff *skb,
 			    nla_parse_nested(tb, IPSET_ATTR_ADT_MAX, nla,
 					     set->type->adt_policy))
 				return -IPSET_ERR_PROTOCOL;
-			ret = call_ad(skb, set, tb, IPSET_DEL,
+			ret = call_ad(ctnl, skb, set, tb, IPSET_DEL,
 				      flags, use_lineno);
 			if (ret < 0)
 				return ret;

[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux