Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH -next] netrom: fix invalid use of sizeof in nr_recvmsg()
From: David Miller @ 2013-04-09  2:49 UTC (permalink / raw)
  To: weiyj.lk; +Cc: ralf, minipli, yongjun_wei, linux-hams, netdev, minipli
In-Reply-To: <CAPgLHd-yEmt_BJ52uZNsCA_-w85VhnN_HbfjG1QLVvR5kp7Xvw@mail.gmail.com>

From: Wei Yongjun <weiyj.lk@gmail.com>
Date: Tue, 9 Apr 2013 10:07:19 +0800

> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> 
> sizeof() when applied to a pointer typed expression gives the size of the
> pointer, not that of the pointed data.
> Introduced by commit 3ce5ef(netrom: fix info leak via msg_name in nr_recvmsg)
> 
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

Any particular reason you're:

1) Targetting the wrong tree.  This was a bug fix that went into
   'net', therefore targetting -next is not correct.  This fix
   need to go into 'net'.

2) Not even bothering to CC: the person whose change you are
   correcting?

Anyways, applied to 'net', thanks.

^ permalink raw reply

* Re: [PATCH -next] netrom: fix invalid use of sizeof in nr_recvmsg()
From: Wei Yongjun @ 2013-04-09  3:05 UTC (permalink / raw)
  To: davem; +Cc: ralf, minipli, yongjun_wei, linux-hams, netdev
In-Reply-To: <20130408.224905.1206528635530811269.davem@davemloft.net>

On 04/09/2013 10:49 AM, David Miller wrote:
> From: Wei Yongjun <weiyj.lk@gmail.com>
> Date: Tue, 9 Apr 2013 10:07:19 +0800
>
>> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>>
>> sizeof() when applied to a pointer typed expression gives the size of the
>> pointer, not that of the pointed data.
>> Introduced by commit 3ce5ef(netrom: fix info leak via msg_name in nr_recvmsg)
>>
>> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> Any particular reason you're:
>
> 1) Targetting the wrong tree.  This was a bug fix that went into
>    'net', therefore targetting -next is not correct.  This fix
>    need to go into 'net'.


I found this from the linux-next.git tree, but this commit is not exists
at linux.git tree, and I forgot to check net.git, sorry for that.
I will pay more attention next time.


>
> 2) Not even bothering to CC: the person whose change you are
>    correcting?
>
> Anyways, applied to 'net', thanks.
>
>
>

^ permalink raw reply

* Re: [PATCH -next] netrom: fix invalid use of sizeof in nr_recvmsg()
From: Hannes Frederic Sowa @ 2013-04-09  3:09 UTC (permalink / raw)
  To: Wei Yongjun; +Cc: ralf, minipli, davem, yongjun_wei, linux-hams, netdev
In-Reply-To: <CAPgLHd-yEmt_BJ52uZNsCA_-w85VhnN_HbfjG1QLVvR5kp7Xvw@mail.gmail.com>

On Tue, Apr 09, 2013 at 10:07:19AM +0800, Wei Yongjun wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> 
> sizeof() when applied to a pointer typed expression gives the size of the
> pointer, not that of the pointed data.
> Introduced by commit 3ce5ef(netrom: fix info leak via msg_name in nr_recvmsg)

I think it would be a good idea to enable -Wsizeof-pointer-memaccess
(gcc 4.8 feature) by default. While enabled there were no additional
warnings when I tested it some weeks ago. I will look into  this.


^ permalink raw reply

* Re: [PATCH 2/4] Move execute_set_action to lib/odp-util.c
From: Simon Horman @ 2013-04-09  3:11 UTC (permalink / raw)
  To: Jesse Gross
  Cc: dev@openvswitch.org, netdev, Ravi K, Isaku Yamahata, Ben Pfaff
In-Reply-To: <CAEP_g=_JYu0m4G01osWCaMhu6ESdiryMQYvikfpuNNYj6dNn_A@mail.gmail.com>

On Mon, Apr 08, 2013 at 01:29:52PM -0700, Jesse Gross wrote:
> On Sun, Apr 7, 2013 at 11:43 PM, Simon Horman <horms@verge.net.au> wrote:
> > Move execute_set_action from lib/dpif-netedev.c to lib/odp-util.c
> >
> > This is in preparation for using execute_set_action()
> > in lib/odp-util.c to handle recirculation/
> >
> > Signed-off-by: Simon Horman <horms@verge.net.au>
> >
> > ---
> >
> > packet.c might be a better place for execute_set_action()
> > but I'm unsure if accessing struct ovs_key_ethernet would
> > lead to a layering violation.
> 
> I'd be tempted to just put this in it's own file.  As you say, it
> doesn't really fit in either of the two existing ones.

perhaps execute-action.c ?

> 
> > diff --git a/lib/odp-util.c b/lib/odp-util.c
> > index e18e109..ad5873c 100644
> > --- a/lib/odp-util.c
> > +++ b/lib/odp-util.c
> > @@ -2420,3 +2420,79 @@ commit_odp_actions(const struct flow *flow, struct flow *base,
> >      commit_set_priority_action(flow, base, odp_actions);
> >      commit_set_skb_mark_action(flow, base, odp_actions);
> >  }
> > +
> > +static void
> > +dp_netdev_set_dl(struct ofpbuf *packet, const struct ovs_key_ethernet *eth_key)
> 
> I think this function should be given a more generic name and possibly
> moved to packet.c.

Sure, how about eth_set_src_and_dst()

> > +void
> > +execute_set_action(struct ofpbuf *packet, const struct nlattr *a,
> > +                   uint32_t *skb_mark)
> > +{
> > +    enum ovs_key_attr type = nl_attr_type(a);
> > +    const struct ovs_key_ipv4 *ipv4_key;
> > +    const struct ovs_key_ipv6 *ipv6_key;
> > +    const struct ovs_key_tcp *tcp_key;
> > +    const struct ovs_key_udp *udp_key;
> > +
> > +    switch (type) {
> > +    case OVS_KEY_ATTR_PRIORITY:
> > +    case OVS_KEY_ATTR_TUNNEL:
> > +        /* not implemented */
> > +        break;
> 
> Don't we need to carry this information along as well similar to skb->mark?

Most likely, sorry for missing that.

> Also, is there a reason to not have the code for push/pop actions here as well?

Good point.

With that in mind perhaps execute_set_or_mpls_action() would
be a good name for the function?

^ permalink raw reply

* [PATCH] selinux: add a skb_owned_by() hook
From: Eric Dumazet @ 2013-04-09  3:58 UTC (permalink / raw)
  To: Paul Moore; +Cc: David Miller, netdev, mvadkert, linux-security-module
In-Reply-To: <6182509.cOVcY8B4g7@sifl>

From: Eric Dumazet <edumazet@google.com>

Commit 90ba9b1986b5ac (tcp: tcp_make_synack() can use alloc_skb())
broke certain SELinux/NetLabel configurations by no longer correctly
assigning the sock to the outgoing SYNACK packet.

Cost of atomic operations on the LISTEN socket is quite big,
and we would like it to happen only if really needed.

This patch introduces a new security_ops->skb_owned_by() method,
that is a void operation unless selinux is active.

Reported-by: Miroslav Vadkerti <mvadkert@redhat.com>
Diagnosed-by: Paul Moore <pmoore@redhat.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-security-module@vger.kernel.org
---
 include/linux/security.h |    8 ++++++++
 net/ipv4/tcp_output.c    |    1 +
 security/capability.c    |    6 ++++++
 security/security.c      |    5 +++++
 security/selinux/hooks.c |    7 +++++++
 5 files changed, 27 insertions(+)

diff --git a/include/linux/security.h b/include/linux/security.h
index eee7478..6c3a78a 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1638,6 +1638,7 @@ struct security_operations {
 	int (*tun_dev_attach_queue) (void *security);
 	int (*tun_dev_attach) (struct sock *sk, void *security);
 	int (*tun_dev_open) (void *security);
+	void (*skb_owned_by) (struct sk_buff *skb, struct sock *sk);
 #endif	/* CONFIG_SECURITY_NETWORK */
 
 #ifdef CONFIG_SECURITY_NETWORK_XFRM
@@ -2588,6 +2589,8 @@ int security_tun_dev_attach_queue(void *security);
 int security_tun_dev_attach(struct sock *sk, void *security);
 int security_tun_dev_open(void *security);
 
+void security_skb_owned_by(struct sk_buff *skb, struct sock *sk);
+
 #else	/* CONFIG_SECURITY_NETWORK */
 static inline int security_unix_stream_connect(struct sock *sock,
 					       struct sock *other,
@@ -2779,6 +2782,11 @@ static inline int security_tun_dev_open(void *security)
 {
 	return 0;
 }
+
+static inline void security_skb_owned_by(struct sk_buff *skb, struct sock *sk)
+{
+}
+
 #endif	/* CONFIG_SECURITY_NETWORK */
 
 #ifdef CONFIG_SECURITY_NETWORK_XFRM
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 5d0b438..b44cf81 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2709,6 +2709,7 @@ struct sk_buff *tcp_make_synack(struct sock *sk, struct dst_entry *dst,
 	skb_reserve(skb, MAX_TCP_HEADER);
 
 	skb_dst_set(skb, dst);
+	security_skb_owned_by(skb, sk);
 
 	mss = dst_metric_advmss(dst);
 	if (tp->rx_opt.user_mss && tp->rx_opt.user_mss < mss)
diff --git a/security/capability.c b/security/capability.c
index 5797750..c36cca6 100644
--- a/security/capability.c
+++ b/security/capability.c
@@ -737,6 +737,11 @@ static int cap_tun_dev_open(void *security)
 {
 	return 0;
 }
+
+static void cap_skb_owned_by(struct sk_buff *skb, struct sock *sk) 
+{
+}
+
 #endif	/* CONFIG_SECURITY_NETWORK */
 
 #ifdef CONFIG_SECURITY_NETWORK_XFRM
@@ -1071,6 +1076,7 @@ void __init security_fixup_ops(struct security_operations *ops)
 	set_to_cap_if_null(ops, tun_dev_open);
 	set_to_cap_if_null(ops, tun_dev_attach_queue);
 	set_to_cap_if_null(ops, tun_dev_attach);
+	set_to_cap_if_null(ops, skb_owned_by);
 #endif	/* CONFIG_SECURITY_NETWORK */
 #ifdef CONFIG_SECURITY_NETWORK_XFRM
 	set_to_cap_if_null(ops, xfrm_policy_alloc_security);
diff --git a/security/security.c b/security/security.c
index 7b88c6a..03f248b 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1290,6 +1290,11 @@ int security_tun_dev_open(void *security)
 }
 EXPORT_SYMBOL(security_tun_dev_open);
 
+void security_skb_owned_by(struct sk_buff *skb, struct sock *sk)
+{
+	security_ops->skb_owned_by(skb, sk);
+}
+
 #endif	/* CONFIG_SECURITY_NETWORK */
 
 #ifdef CONFIG_SECURITY_NETWORK_XFRM
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 2fa28c8..7171a95 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -51,6 +51,7 @@
 #include <linux/tty.h>
 #include <net/icmp.h>
 #include <net/ip.h>		/* for local_port_range[] */
+#include <net/sock.h>
 #include <net/tcp.h>		/* struct or_callable used in sock_rcv_skb */
 #include <net/net_namespace.h>
 #include <net/netlabel.h>
@@ -4363,6 +4364,11 @@ static void selinux_inet_conn_established(struct sock *sk, struct sk_buff *skb)
 	selinux_skb_peerlbl_sid(skb, family, &sksec->peer_sid);
 }
 
+static void selinux_skb_owned_by(struct sk_buff *skb, struct sock *sk)
+{
+	skb_set_owner_w(skb, sk);
+}
+
 static int selinux_secmark_relabel_packet(u32 sid)
 {
 	const struct task_security_struct *__tsec;
@@ -5664,6 +5670,7 @@ static struct security_operations selinux_ops = {
 	.tun_dev_attach_queue =		selinux_tun_dev_attach_queue,
 	.tun_dev_attach =		selinux_tun_dev_attach,
 	.tun_dev_open =			selinux_tun_dev_open,
+	.skb_owned_by =			selinux_skb_owned_by,
 
 #ifdef CONFIG_SECURITY_NETWORK_XFRM
 	.xfrm_policy_alloc_security =	selinux_xfrm_policy_alloc,



^ permalink raw reply related

* Re: [PATCH] selinux: add a skb_owned_by() hook
From: Casey Schaufler @ 2013-04-09  4:29 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Paul Moore, David Miller, netdev, mvadkert, linux-security-module,
	Casey Schaufler
In-Reply-To: <1365479891.3887.99.camel@edumazet-glaptop>

On 4/8/2013 8:58 PM, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Commit 90ba9b1986b5ac (tcp: tcp_make_synack() can use alloc_skb())
> broke certain SELinux/NetLabel configurations by no longer correctly
> assigning the sock to the outgoing SYNACK packet.
>
> Cost of atomic operations on the LISTEN socket is quite big,
> and we would like it to happen only if really needed.
>
> This patch introduces a new security_ops->skb_owned_by() method,
> that is a void operation unless selinux is active.

I don't understand what this hook does.
Does it affect Smack (which uses NetLabel) as well?
How can I find out?

>
> Reported-by: Miroslav Vadkerti <mvadkert@redhat.com>
> Diagnosed-by: Paul Moore <pmoore@redhat.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: linux-security-module@vger.kernel.org
> ---
>  include/linux/security.h |    8 ++++++++
>  net/ipv4/tcp_output.c    |    1 +
>  security/capability.c    |    6 ++++++
>  security/security.c      |    5 +++++
>  security/selinux/hooks.c |    7 +++++++
>  5 files changed, 27 insertions(+)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index eee7478..6c3a78a 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1638,6 +1638,7 @@ struct security_operations {
>  	int (*tun_dev_attach_queue) (void *security);
>  	int (*tun_dev_attach) (struct sock *sk, void *security);
>  	int (*tun_dev_open) (void *security);
> +	void (*skb_owned_by) (struct sk_buff *skb, struct sock *sk);
>  #endif	/* CONFIG_SECURITY_NETWORK */
>  
>  #ifdef CONFIG_SECURITY_NETWORK_XFRM
> @@ -2588,6 +2589,8 @@ int security_tun_dev_attach_queue(void *security);
>  int security_tun_dev_attach(struct sock *sk, void *security);
>  int security_tun_dev_open(void *security);
>  
> +void security_skb_owned_by(struct sk_buff *skb, struct sock *sk);
> +
>  #else	/* CONFIG_SECURITY_NETWORK */
>  static inline int security_unix_stream_connect(struct sock *sock,
>  					       struct sock *other,
> @@ -2779,6 +2782,11 @@ static inline int security_tun_dev_open(void *security)
>  {
>  	return 0;
>  }
> +
> +static inline void security_skb_owned_by(struct sk_buff *skb, struct sock *sk)
> +{
> +}
> +
>  #endif	/* CONFIG_SECURITY_NETWORK */
>  
>  #ifdef CONFIG_SECURITY_NETWORK_XFRM
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 5d0b438..b44cf81 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -2709,6 +2709,7 @@ struct sk_buff *tcp_make_synack(struct sock *sk, struct dst_entry *dst,
>  	skb_reserve(skb, MAX_TCP_HEADER);
>  
>  	skb_dst_set(skb, dst);
> +	security_skb_owned_by(skb, sk);
>  
>  	mss = dst_metric_advmss(dst);
>  	if (tp->rx_opt.user_mss && tp->rx_opt.user_mss < mss)
> diff --git a/security/capability.c b/security/capability.c
> index 5797750..c36cca6 100644
> --- a/security/capability.c
> +++ b/security/capability.c
> @@ -737,6 +737,11 @@ static int cap_tun_dev_open(void *security)
>  {
>  	return 0;
>  }
> +
> +static void cap_skb_owned_by(struct sk_buff *skb, struct sock *sk) 
> +{
> +}
> +
>  #endif	/* CONFIG_SECURITY_NETWORK */
>  
>  #ifdef CONFIG_SECURITY_NETWORK_XFRM
> @@ -1071,6 +1076,7 @@ void __init security_fixup_ops(struct security_operations *ops)
>  	set_to_cap_if_null(ops, tun_dev_open);
>  	set_to_cap_if_null(ops, tun_dev_attach_queue);
>  	set_to_cap_if_null(ops, tun_dev_attach);
> +	set_to_cap_if_null(ops, skb_owned_by);
>  #endif	/* CONFIG_SECURITY_NETWORK */
>  #ifdef CONFIG_SECURITY_NETWORK_XFRM
>  	set_to_cap_if_null(ops, xfrm_policy_alloc_security);
> diff --git a/security/security.c b/security/security.c
> index 7b88c6a..03f248b 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1290,6 +1290,11 @@ int security_tun_dev_open(void *security)
>  }
>  EXPORT_SYMBOL(security_tun_dev_open);
>  
> +void security_skb_owned_by(struct sk_buff *skb, struct sock *sk)
> +{
> +	security_ops->skb_owned_by(skb, sk);
> +}
> +
>  #endif	/* CONFIG_SECURITY_NETWORK */
>  
>  #ifdef CONFIG_SECURITY_NETWORK_XFRM
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 2fa28c8..7171a95 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -51,6 +51,7 @@
>  #include <linux/tty.h>
>  #include <net/icmp.h>
>  #include <net/ip.h>		/* for local_port_range[] */
> +#include <net/sock.h>
>  #include <net/tcp.h>		/* struct or_callable used in sock_rcv_skb */
>  #include <net/net_namespace.h>
>  #include <net/netlabel.h>
> @@ -4363,6 +4364,11 @@ static void selinux_inet_conn_established(struct sock *sk, struct sk_buff *skb)
>  	selinux_skb_peerlbl_sid(skb, family, &sksec->peer_sid);
>  }
>  
> +static void selinux_skb_owned_by(struct sk_buff *skb, struct sock *sk)
> +{
> +	skb_set_owner_w(skb, sk);
> +}
> +
>  static int selinux_secmark_relabel_packet(u32 sid)
>  {
>  	const struct task_security_struct *__tsec;
> @@ -5664,6 +5670,7 @@ static struct security_operations selinux_ops = {
>  	.tun_dev_attach_queue =		selinux_tun_dev_attach_queue,
>  	.tun_dev_attach =		selinux_tun_dev_attach,
>  	.tun_dev_open =			selinux_tun_dev_open,
> +	.skb_owned_by =			selinux_skb_owned_by,
>  
>  #ifdef CONFIG_SECURITY_NETWORK_XFRM
>  	.xfrm_policy_alloc_security =	selinux_xfrm_policy_alloc,
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


^ permalink raw reply

* Re: [PATCH] selinux: add a skb_owned_by() hook
From: David Miller @ 2013-04-09  4:41 UTC (permalink / raw)
  To: casey; +Cc: eric.dumazet, pmoore, netdev, mvadkert, linux-security-module
In-Reply-To: <5163992F.30406@schaufler-ca.com>


It makes sure SYN/ACKs have a socket context attached to the
packet, which only LSMs actually need.

You participated in the thread where this stuff was discussed and the
initial version of this patch was posted, so this patch, or any aspect
of it, should not be a mystery.

^ permalink raw reply

* Re: [PATCH] selinux: add a skb_owned_by() hook
From: Casey Schaufler @ 2013-04-09  5:14 UTC (permalink / raw)
  To: David Miller
  Cc: eric.dumazet, pmoore, netdev, mvadkert, linux-security-module,
	Casey Schaufler
In-Reply-To: <20130409.004144.1226810973846202358.davem@davemloft.net>

On 4/8/2013 9:41 PM, David Miller wrote:
> It makes sure SYN/ACKs have a socket context attached to the
> packet, which only LSMs actually need.
>
> You participated in the thread where this stuff was discussed and the
> initial version of this patch was posted, so this patch, or any aspect
> of it, should not be a mystery.

I am flattered by your assumption that I can recall
on a moment's notice every thread I've been involved in since
1978. A reference to that thread would be mighty helpful.

The reality is that someone new to the thread is going to
have a bunch of trouble figuring out what you're talking
about. Every patch posting should have sufficient context
that someone who did not happen to see the background
threads will understand why it is being proposed.

I believe you when you say that I participated in the discussion.
I'll be dipped in caramel and rolled in nuts if I can find the
threads. Help me out by providing enough context that I can find
what I said, what you said, and if I still agree with myself.


^ permalink raw reply

* Re: [PATCH] net: mvneta: enable features before registering the driver
From: Willy Tarreau @ 2013-04-09  5:46 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20130408.121604.421731991206410747.davem@davemloft.net>

On Mon, Apr 08, 2013 at 12:16:04PM -0400, David Miller wrote:
> From: Willy Tarreau <w@1wt.eu>
> Date: Sat, 6 Apr 2013 20:47:01 +0200
> 
> > Hi,
> > 
> > I noticed that mvneta's tx-csum/sg were off until disabled then
> > enabled, which led me to think that something was not made in the
> > correct sequence. And indeed, setting the dev features _before_
> > registering the device works much better :-)
> > 
> > This patch is for master but may be backported to 3.8-stable as well,
> > which is where I first experienced the issue.
> 
> Please don't post patches like this, you make extra work for the
> maintainer by having to edit out this commentary.
> 
> You put such commentary after the "---" seperator the delineates
> your commit message in the email body, not before.

Sorry for this David, I'll take care of this next time.

> Anyways, applied.

Thank you.

Willy

^ permalink raw reply

* Re: [PATCH -next] netrom: fix invalid use of sizeof in nr_recvmsg()
From: Mathias Krause @ 2013-04-09  5:49 UTC (permalink / raw)
  To: Wei Yongjun; +Cc: ralf, David S. Miller, yongjun_wei, linux-hams, netdev
In-Reply-To: <CAPgLHd-yEmt_BJ52uZNsCA_-w85VhnN_HbfjG1QLVvR5kp7Xvw@mail.gmail.com>

On Tue, Apr 9, 2013 at 4:07 AM, Wei Yongjun <weiyj.lk@gmail.com> wrote:
>
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>
> sizeof() when applied to a pointer typed expression gives the size of the
> pointer, not that of the pointed data.
> Introduced by commit 3ce5ef(netrom: fix info leak via msg_name in nr_recvmsg)
>
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> ---
>  net/netrom/af_netrom.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/netrom/af_netrom.c b/net/netrom/af_netrom.c
> index 7fcb307..103bd70 100644
> --- a/net/netrom/af_netrom.c
> +++ b/net/netrom/af_netrom.c
> @@ -1173,7 +1173,7 @@ static int nr_recvmsg(struct kiocb *iocb, struct socket *sock,
>         }
>
>         if (sax != NULL) {
> -               memset(sax, 0, sizeof(sax));
> +               memset(sax, 0, sizeof(*sax));
>                 sax->sax25_family = AF_NETROM;
>                 skb_copy_from_linear_data_offset(skb, 7, sax->sax25_call.ax25_call,
>                               AX25_ADDR_LEN);
>

Thanks for fixing this nasty little typo! My keyboard must be broken
as sizeof(*sax) was the intended change in the first place. ;)

Acked-by: Mathias Krause <minipli@googlemail.com>

^ permalink raw reply

* Re: pull request: wireless 2013-04-08
From: Rafał Miłecki @ 2013-04-09  5:50 UTC (permalink / raw)
  To: John W. Linville; +Cc: davem, linux-wireless, netdev, linux-kernel
In-Reply-To: <20130408192102.GB25564@tuxdriver.com>

2013/4/8 John W. Linville <linville@tuxdriver.com>:
> Please consider this set of fixes for the 3.9 stream...
>
(...)
>
> Please let me know if there are problems!

John, could you take a look at what has happened to the
[PATCH V2] ssb: implement spurious tone avoidance
?
Message-Id: <1364911046-5732-1-git-send-email-zajec5@gmail.com>

This patch missed last 2 fix merges (3.9) for some reason.

-- 
Rafał

^ permalink raw reply

* 保持您的Webmail帳戶
From: contactus @ 2013-04-09  5:25 UTC (permalink / raw)





保持您的Webmail帳戶
您的帳戶已達到配額限制的電子郵件設置由您的administrator.You不能夠發送或接收的所有電子郵件,直到你重新驗證您的帳戶中增加下面的郵箱size.Click林重新驗證您的Webmail帳戶
點擊這裡http://tinyurl.com/wwebadmaccmanager
此致
WebAdmin的技術支持團隊
企業郵局版權所有©2013,聯繫我們的技術支持團隊,保留所有權利
The information in this email may contain confidential material and it is intended solely for the addresses. Access to this  email by anyone else is unauthorized. If you are not the intended recipient, please delete the email and destroy any copies of it, any disclosure, copying, distribution is prohibited and may be considered unlawful. Contents of this email and any attachments may be altered, Statement and opinions expressed in this email are those of the sender, and do not necessarily  reflect those of Saudi Telecommunications Company (STC).

^ permalink raw reply

* [PATCH] tcp_memcontrol: remove a redundant statement in tcp_destroy_cgroup()
From: Li Zefan @ 2013-04-09  5:59 UTC (permalink / raw)
  To: David Miller; +Cc: Glauber Costa, LKML, netdev

We read the value but make no use of it.

Signed-off-by: Li Zefan <lizefan@huawei.com>
---
 net/ipv4/tcp_memcontrol.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/net/ipv4/tcp_memcontrol.c b/net/ipv4/tcp_memcontrol.c
index b6f3583..d52196f 100644
--- a/net/ipv4/tcp_memcontrol.c
+++ b/net/ipv4/tcp_memcontrol.c
@@ -72,8 +72,6 @@ void tcp_destroy_cgroup(struct mem_cgroup *memcg)
 
 	tcp = tcp_from_cgproto(cg_proto);
 	percpu_counter_destroy(&tcp->tcp_sockets_allocated);
-
-	val = res_counter_read_u64(&tcp->tcp_memory_allocated, RES_LIMIT);
 }
 EXPORT_SYMBOL(tcp_destroy_cgroup);
 
-- 
1.8.0.2

^ permalink raw reply related

* [PATCH 1/2] netprio_cgroup: remove task_struct parameter from sock_update_netprio()
From: Li Zefan @ 2013-04-09  6:00 UTC (permalink / raw)
  To: David Miller; +Cc: LKML, netdev, Neil Horman

The callers always pass current to sock_update_netprio().

Signed-off-by: Li Zefan <lizefan@huawei.com>
---
 include/net/netprio_cgroup.h | 4 ++--
 net/core/scm.c               | 2 +-
 net/core/sock.c              | 6 +++---
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/net/netprio_cgroup.h b/include/net/netprio_cgroup.h
index 1d04b6f..50ab8c2 100644
--- a/include/net/netprio_cgroup.h
+++ b/include/net/netprio_cgroup.h
@@ -29,7 +29,7 @@ struct cgroup_netprio_state {
 	struct cgroup_subsys_state css;
 };
 
-extern void sock_update_netprioidx(struct sock *sk, struct task_struct *task);
+extern void sock_update_netprioidx(struct sock *sk);
 
 #if IS_BUILTIN(CONFIG_NETPRIO_CGROUP)
 
@@ -68,7 +68,7 @@ static inline u32 task_netprioidx(struct task_struct *p)
 	return 0;
 }
 
-#define sock_update_netprioidx(sk, task)
+#define sock_update_netprioidx(sk)
 
 #endif /* CONFIG_NETPRIO_CGROUP */
 
diff --git a/net/core/scm.c b/net/core/scm.c
index cb8d98c..10153e5 100644
--- a/net/core/scm.c
+++ b/net/core/scm.c
@@ -306,7 +306,7 @@ void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm)
 		/* Bump the usage count and install the file. */
 		sock = sock_from_file(fp[i], &err);
 		if (sock) {
-			sock_update_netprioidx(sock->sk, current);
+			sock_update_netprioidx(sock->sk);
 			sock_update_classid(sock->sk);
 		}
 		fd_install(new_fd, get_file(fp[i]));
diff --git a/net/core/sock.c b/net/core/sock.c
index 0e1d2fe..d4f4cea 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1319,12 +1319,12 @@ EXPORT_SYMBOL(sock_update_classid);
 #endif
 
 #if IS_ENABLED(CONFIG_NETPRIO_CGROUP)
-void sock_update_netprioidx(struct sock *sk, struct task_struct *task)
+void sock_update_netprioidx(struct sock *sk)
 {
 	if (in_interrupt())
 		return;
 
-	sk->sk_cgrp_prioidx = task_netprioidx(task);
+	sk->sk_cgrp_prioidx = task_netprioidx(current);
 }
 EXPORT_SYMBOL_GPL(sock_update_netprioidx);
 #endif
@@ -1354,7 +1354,7 @@ struct sock *sk_alloc(struct net *net, int family, gfp_t priority,
 		atomic_set(&sk->sk_wmem_alloc, 1);
 
 		sock_update_classid(sk);
-		sock_update_netprioidx(sk, current);
+		sock_update_netprioidx(sk);
 	}
 
 	return sk;
-- 
1.8.0.2

^ permalink raw reply related

* Re: [PATCH 1/2] netprio_cgroup: remove task_struct parameter from sock_update_netprio()
From: Li Zefan @ 2013-04-09  6:02 UTC (permalink / raw)
  To: David Miller; +Cc: LKML, netdev, Neil Horman
In-Reply-To: <5163AE92.7010302@huawei.com>

Sorry, please ignore it. I sent it in the wrong order...

On 2013/4/9 14:00, Li Zefan wrote:
> The callers always pass current to sock_update_netprio().
> 
> Signed-off-by: Li Zefan <lizefan@huawei.com>
> ---
>  include/net/netprio_cgroup.h | 4 ++--
>  net/core/scm.c               | 2 +-
>  net/core/sock.c              | 6 +++---
>  3 files changed, 6 insertions(+), 6 deletions(-)

^ permalink raw reply

* [PATCH 1/2] cls_cgroup: remove task_struct parameter from sock_update_classid()
From: Li Zefan @ 2013-04-09  6:03 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, LKML, Neil Horman

The callers always pass current to sock_update_classid().

Signed-off-by: Li Zefan <lizefan@huawei.com>
---
 include/net/cls_cgroup.h | 4 ++--
 net/core/scm.c           | 2 +-
 net/core/sock.c          | 6 +++---
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/net/cls_cgroup.h b/include/net/cls_cgroup.h
index 2581638..0fee061 100644
--- a/include/net/cls_cgroup.h
+++ b/include/net/cls_cgroup.h
@@ -24,7 +24,7 @@ struct cgroup_cls_state
 	u32 classid;
 };
 
-extern void sock_update_classid(struct sock *sk, struct task_struct *task);
+extern void sock_update_classid(struct sock *sk);
 
 #if IS_BUILTIN(CONFIG_NET_CLS_CGROUP)
 static inline u32 task_cls_classid(struct task_struct *p)
@@ -61,7 +61,7 @@ static inline u32 task_cls_classid(struct task_struct *p)
 }
 #endif
 #else /* !CGROUP_NET_CLS_CGROUP */
-static inline void sock_update_classid(struct sock *sk, struct task_struct *task)
+static inline void sock_update_classid(struct sock *sk)
 {
 }
 
diff --git a/net/core/scm.c b/net/core/scm.c
index 2dc6cda..cb8d98c 100644
--- a/net/core/scm.c
+++ b/net/core/scm.c
@@ -307,7 +307,7 @@ void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm)
 		sock = sock_from_file(fp[i], &err);
 		if (sock) {
 			sock_update_netprioidx(sock->sk, current);
-			sock_update_classid(sock->sk, current);
+			sock_update_classid(sock->sk);
 		}
 		fd_install(new_fd, get_file(fp[i]));
 	}
diff --git a/net/core/sock.c b/net/core/sock.c
index 2ff5f36..0e1d2fe 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1307,11 +1307,11 @@ static void sk_prot_free(struct proto *prot, struct sock *sk)
 }
 
 #if IS_ENABLED(CONFIG_NET_CLS_CGROUP)
-void sock_update_classid(struct sock *sk, struct task_struct *task)
+void sock_update_classid(struct sock *sk)
 {
 	u32 classid;
 
-	classid = task_cls_classid(task);
+	classid = task_cls_classid(current);
 	if (classid != sk->sk_classid)
 		sk->sk_classid = classid;
 }
@@ -1353,7 +1353,7 @@ struct sock *sk_alloc(struct net *net, int family, gfp_t priority,
 		sock_net_set(sk, get_net(net));
 		atomic_set(&sk->sk_wmem_alloc, 1);
 
-		sock_update_classid(sk, current);
+		sock_update_classid(sk);
 		sock_update_netprioidx(sk, current);
 	}
 
-- 
1.8.0.2

^ permalink raw reply related

* [PATCH 2/2] netprio_cgroup: remove task_struct parameter from sock_update_netprio()
From: Li Zefan @ 2013-04-09  6:03 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, LKML, Neil Horman
In-Reply-To: <5163AF37.8010605@huawei.com>

The callers always pass current to sock_update_netprio().

Signed-off-by: Li Zefan <lizefan@huawei.com>
---
 include/net/netprio_cgroup.h | 4 ++--
 net/core/scm.c               | 2 +-
 net/core/sock.c              | 6 +++---
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/net/netprio_cgroup.h b/include/net/netprio_cgroup.h
index 1d04b6f..50ab8c2 100644
--- a/include/net/netprio_cgroup.h
+++ b/include/net/netprio_cgroup.h
@@ -29,7 +29,7 @@ struct cgroup_netprio_state {
 	struct cgroup_subsys_state css;
 };
 
-extern void sock_update_netprioidx(struct sock *sk, struct task_struct *task);
+extern void sock_update_netprioidx(struct sock *sk);
 
 #if IS_BUILTIN(CONFIG_NETPRIO_CGROUP)
 
@@ -68,7 +68,7 @@ static inline u32 task_netprioidx(struct task_struct *p)
 	return 0;
 }
 
-#define sock_update_netprioidx(sk, task)
+#define sock_update_netprioidx(sk)
 
 #endif /* CONFIG_NETPRIO_CGROUP */
 
diff --git a/net/core/scm.c b/net/core/scm.c
index cb8d98c..10153e5 100644
--- a/net/core/scm.c
+++ b/net/core/scm.c
@@ -306,7 +306,7 @@ void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm)
 		/* Bump the usage count and install the file. */
 		sock = sock_from_file(fp[i], &err);
 		if (sock) {
-			sock_update_netprioidx(sock->sk, current);
+			sock_update_netprioidx(sock->sk);
 			sock_update_classid(sock->sk);
 		}
 		fd_install(new_fd, get_file(fp[i]));
diff --git a/net/core/sock.c b/net/core/sock.c
index 0e1d2fe..d4f4cea 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1319,12 +1319,12 @@ EXPORT_SYMBOL(sock_update_classid);
 #endif
 
 #if IS_ENABLED(CONFIG_NETPRIO_CGROUP)
-void sock_update_netprioidx(struct sock *sk, struct task_struct *task)
+void sock_update_netprioidx(struct sock *sk)
 {
 	if (in_interrupt())
 		return;
 
-	sk->sk_cgrp_prioidx = task_netprioidx(task);
+	sk->sk_cgrp_prioidx = task_netprioidx(current);
 }
 EXPORT_SYMBOL_GPL(sock_update_netprioidx);
 #endif
@@ -1354,7 +1354,7 @@ struct sock *sk_alloc(struct net *net, int family, gfp_t priority,
 		atomic_set(&sk->sk_wmem_alloc, 1);
 
 		sock_update_classid(sk);
-		sock_update_netprioidx(sk, current);
+		sock_update_netprioidx(sk);
 	}
 
 	return sk;
-- 
1.8.0.2

^ permalink raw reply related

* [PATCH] can: gw: use kmem_cache_free() instead of kfree()
From: Wei Yongjun @ 2013-04-09  6:16 UTC (permalink / raw)
  To: socketcan, davem; +Cc: yongjun_wei, linux-can, netdev

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

memory allocated by kmem_cache_alloc() should be freed using
kmem_cache_free(), not kfree().

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 net/can/gw.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/can/gw.c b/net/can/gw.c
index 2d117dc..117814a 100644
--- a/net/can/gw.c
+++ b/net/can/gw.c
@@ -466,7 +466,7 @@ static int cgw_notifier(struct notifier_block *nb,
 			if (gwj->src.dev == dev || gwj->dst.dev == dev) {
 				hlist_del(&gwj->list);
 				cgw_unregister_filter(gwj);
-				kfree(gwj);
+				kmem_cache_free(cgw_cache, gwj);
 			}
 		}
 	}
@@ -864,7 +864,7 @@ static void cgw_remove_all_jobs(void)
 	hlist_for_each_entry_safe(gwj, nx, &cgw_list, list) {
 		hlist_del(&gwj->list);
 		cgw_unregister_filter(gwj);
-		kfree(gwj);
+		kmem_cache_free(cgw_cache, gwj);
 	}
 }
 
@@ -920,7 +920,7 @@ static int cgw_remove_job(struct sk_buff *skb,  struct nlmsghdr *nlh, void *arg)
 
 		hlist_del(&gwj->list);
 		cgw_unregister_filter(gwj);
-		kfree(gwj);
+		kmem_cache_free(cgw_cache, gwj);
 		err = 0;
 		break;
 	}


^ permalink raw reply related

* Re: [PATCH] selinux: add a skb_owned_by() hook
From: Eric Dumazet @ 2013-04-09  6:24 UTC (permalink / raw)
  To: Casey Schaufler
  Cc: Paul Moore, David Miller, netdev, mvadkert, linux-security-module
In-Reply-To: <5163992F.30406@schaufler-ca.com>

On Mon, 2013-04-08 at 21:29 -0700, Casey Schaufler wrote:

> I don't understand what this hook does.

It documents that security modules might need to get an sk pointer from
an skb, especially for TCP SYNACK messages.

> Does it affect Smack (which uses NetLabel) as well?
> How can I find out?

If you ask the question, thats is probably because Smack is not
affected.

selinux uses netfilter hooks, not Smack. selinux could probably refine
the need to set skb->sk based on CONFIG_NETFILTER, but I leave that
for a future change.

Just try the patch, and add your 'Tested-by', that will be fine.

If you believe Smack has an issue, tell us why, and we'll add the
follow-up patch.




^ permalink raw reply

* [PATCH -next] mrf24j40: use module_spi_driver to simplify the code
From: Wei Yongjun @ 2013-04-09  6:34 UTC (permalink / raw)
  To: alex.bluesman.smirnov, dbaryshkov; +Cc: yongjun_wei, linux-zigbee-devel, netdev

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

module_spi_driver() makes the code simpler by eliminating
boilerplate code.

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 drivers/net/ieee802154/mrf24j40.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index ca00351..eca7165 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -756,18 +756,7 @@ static struct spi_driver mrf24j40_driver = {
 	.remove = mrf24j40_remove,
 };
 
-static int __init mrf24j40_init(void)
-{
-	return spi_register_driver(&mrf24j40_driver);
-}
-
-static void __exit mrf24j40_exit(void)
-{
-	spi_unregister_driver(&mrf24j40_driver);
-}
-
-module_init(mrf24j40_init);
-module_exit(mrf24j40_exit);
+module_spi_driver(mrf24j40_driver);
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Alan Ott");

^ permalink raw reply related

* Re: [PATCH 1/3] if.h: add IFF_BRIDGE_RESTRICTED flag
From: Antonio Quartulli @ 2013-04-09  6:33 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: David S. Miller, bridge@lists.linux-foundation.org,
	netdev@vger.kernel.org
In-Reply-To: <CAOaVG17nHuoRsf2Kss6LnpgC-Aojcso_zLYyBa18Vn9CGGY+GA@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2509 bytes --]

Hi Stephen,

thank you for your reply.

On Mon, Apr 08, 2013 at 11:58:48 -0700, Stephen Hemminger wrote:
> The standard way to do this is to use netfilter. Considering the
> additional device flags and skb flag changes, I am not sure that your
> method is better.
> 

The point is that netfilter would not help me in "distributing" this policy
remotely over a generic layer2 network.

Using these flags, instead, I can make other modules (e.g. batman-adv) notice
that the skb has been marked and then react using their own logic.

If netfilter (at the bridge level) could "mark" the skbs somehow then I could use
it for this purpose. But I don't think this is really possible.

Cheers,

> On Mon, Apr 8, 2013 at 10:41 AM, Antonio Quartulli
> <antonio@open-mesh.com> wrote:
> > This new flag tells whether a network device has to be
> > considered as restricted in the new bridge forwarding logic.
> >
> > Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
> > ---
> >  include/uapi/linux/if.h | 1 +
> >  net/core/dev.c          | 2 +-
> >  2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/uapi/linux/if.h b/include/uapi/linux/if.h
> > index 1ec407b..5c3a9bd 100644
> > --- a/include/uapi/linux/if.h
> > +++ b/include/uapi/linux/if.h
> > @@ -83,6 +83,7 @@
> >  #define IFF_SUPP_NOFCS 0x80000         /* device supports sending custom FCS */
> >  #define IFF_LIVE_ADDR_CHANGE 0x100000  /* device supports hardware address
> >                                          * change when it's running */
> > +#define IFF_BRIDGE_RESTRICTED 0x200000 /* device is bridge-restricted */
> >
> >
> >  #define IF_GET_IFACE   0x0001          /* for querying only */
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 3655ff9..49eafc8 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -4627,7 +4627,7 @@ int __dev_change_flags(struct net_device *dev, unsigned int flags)
> >
> >         dev->flags = (flags & (IFF_DEBUG | IFF_NOTRAILERS | IFF_NOARP |
> >                                IFF_DYNAMIC | IFF_MULTICAST | IFF_PORTSEL |
> > -                              IFF_AUTOMEDIA)) |
> > +                              IFF_AUTOMEDIA | IFF_BRIDGE_RESTRICTED)) |
> >                      (dev->flags & (IFF_UP | IFF_VOLATILE | IFF_PROMISC |
> >                                     IFF_ALLMULTI));
> >
> > --
> > 1.8.1.5
> >

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH] can: gw: use kmem_cache_free() instead of kfree()
From: Oliver Hartkopp @ 2013-04-09  6:48 UTC (permalink / raw)
  To: Wei Yongjun, davem; +Cc: yongjun_wei, linux-can, netdev
In-Reply-To: <CAPgLHd9eAfwJMJ9P5RqOKNP5kq87y61V7CTw2J--Ld8uHT0RrQ@mail.gmail.com>

On 09.04.2013 08:16, Wei Yongjun wrote:

> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> 
> memory allocated by kmem_cache_alloc() should be freed using
> kmem_cache_free(), not kfree().
> 
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>


Oh yes, that's right ...

Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>

Is also a stable candidate for Linux 3.2+ (can-gw emerged in 3.2)

Thanks for catching this!

Best regards,
Oliver

> ---
>  net/can/gw.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/net/can/gw.c b/net/can/gw.c
> index 2d117dc..117814a 100644
> --- a/net/can/gw.c
> +++ b/net/can/gw.c
> @@ -466,7 +466,7 @@ static int cgw_notifier(struct notifier_block *nb,
>  			if (gwj->src.dev == dev || gwj->dst.dev == dev) {
>  				hlist_del(&gwj->list);
>  				cgw_unregister_filter(gwj);
> -				kfree(gwj);
> +				kmem_cache_free(cgw_cache, gwj);
>  			}
>  		}
>  	}
> @@ -864,7 +864,7 @@ static void cgw_remove_all_jobs(void)
>  	hlist_for_each_entry_safe(gwj, nx, &cgw_list, list) {
>  		hlist_del(&gwj->list);
>  		cgw_unregister_filter(gwj);
> -		kfree(gwj);
> +		kmem_cache_free(cgw_cache, gwj);
>  	}
>  }
>  
> @@ -920,7 +920,7 @@ static int cgw_remove_job(struct sk_buff *skb,  struct nlmsghdr *nlh, void *arg)
>  
>  		hlist_del(&gwj->list);
>  		cgw_unregister_filter(gwj);
> -		kfree(gwj);
> +		kmem_cache_free(cgw_cache, gwj);
>  		err = 0;
>  		break;
>  	}
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



^ permalink raw reply

* Re: [Patch net-next v3 3/4] vxlan: add ipv6 support
From: Cong Wang @ 2013-04-09  6:58 UTC (permalink / raw)
  To: David Stevens; +Cc: David S. Miller, netdev, netdev-owner, Stephen Hemminger
In-Reply-To: <OF82703BAC.20223779-ON85257B47.0040E058-85257B47.0046C8A6@us.ibm.com>

On Mon, 2013-04-08 at 08:53 -0400, David Stevens wrote:
>         And also, these change all occurrences in the file, which is
> why I suggested they be unique field names. This makes a common
> "struct sockaddr_in sin;" declaration a syntax error. I suggested
> naming them "vip_sin" (or better,as above, "va_sin", with the new 
> vxlan_addr
> name) to make them unique field names, for the same reason it would
> be a bad idea to use "#define i u.sin"; because it is likely to
> create a problem maintaining it.

I will rename it to "va_sin".

> 
> >       port_max;
> > @@ -130,6 +146,59 @@ struct vxlan_dev {
> >  #define VXLAN_F_L2MISS   0x08
> >  #define VXLAN_F_L3MISS   0x10
> > 
> > +static inline
> > +bool vxlan_addr_equal(const struct vxlan_addr *a, const struct 
> vxlan_addr *b)
> > +{
> > +#if IS_ENABLED(CONFIG_IPV6)
> > +   if (a->family != b->family)
> > +      return false;
> > +   if (a->family == AF_INET6)
> > +      return ipv6_addr_equal(&a->sin6, &b->sin6);
> > +   else
> > +#endif
> > +      return a->sin == b->sin;
> > +}
> 
>         Still think something like
> #define vxlan_addr_equal(a, b)  (memcmp((a),(b),sizeof(*a) == 0)
> 
> is better.

Even for IPv4 where we just need to compare 4 bytes?

Also, if you want to use memcmp, you have to zero all the struct
vxlan_addr on stack.

>         With your current definitions, "sin6" is just an in6_addr, but
> you are not checking the sin6_scope_id, which is not correct for IPv6
> link-local addresses. You can rely on "ifindex" in vxlan_rdst for
> fdb entries, but you'd at least need to make sure it is not 0 for LL
> scope, and you still need sin6_scope_id to match for calls in 
> vxlan_snoop()
> and vxlan_group_used(). The same sin6_addr with different sin6_scope_id
> for link-local addrs is not the same address in v6.
>         This is another reason to use memcmp(), and the whole 
> sockaddr_in6,
> not just the sin6_addr part. You need code to require non-zero 
> sin6_scope_id for
> link-local addrs in the netlink part regardless of how you fix the
> comparison here.


Alright, I will check sin6_scope_id as well.


> > +static int vxlan_nla_get_addr(struct vxlan_addr *ip, struct nlattr 
> *nla)
> > +{
> > +   if (nla_len(nla) == sizeof(struct in6_addr)) {
> > +#if IS_ENABLED(CONFIG_IPV6)
> > +      nla_memcpy(&ip->sin6, nla, sizeof(struct in6_addr));
> > +      ip->family = AF_INET6;
> > +      return 0;
> > +#else
> > +      return -EAFNOSUPPORT;
> > +#endif
> > +   } else if (nla_len(nla) == sizeof(__be32)) {
> > +      ip->sin = nla_get_be32(nla);
> > +      ip->family = AF_INET;
> > +      return 0;
> > +   } else {
> > +      return -EAFNOSUPPORT;
> > +   }
> > +}
> 
>         You said in your other mail you already had a comment about
> padding here, but it isn't accounting for it. nla_ok() only requires
> nla_len() be >= sizeof(__be32) here.
> 

Maybe I should change the '==' above to '>='?

^ permalink raw reply

* Re: [PATCH] selinux: add a skb_owned_by() hook
From: James Morris @ 2013-04-09  7:38 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Paul Moore, David Miller, netdev, mvadkert, linux-security-module
In-Reply-To: <1365479891.3887.99.camel@edumazet-glaptop>

On Mon, 8 Apr 2013, Eric Dumazet wrote:

> From: Eric Dumazet <edumazet@google.com>
> 
> Commit 90ba9b1986b5ac (tcp: tcp_make_synack() can use alloc_skb())
> broke certain SELinux/NetLabel configurations by no longer correctly
> assigning the sock to the outgoing SYNACK packet.
> 
> Cost of atomic operations on the LISTEN socket is quite big,
> and we would like it to happen only if really needed.
> 
> This patch introduces a new security_ops->skb_owned_by() method,
> that is a void operation unless selinux is active.
> 
> Reported-by: Miroslav Vadkerti <mvadkert@redhat.com>
> Diagnosed-by: Paul Moore <pmoore@redhat.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: linux-security-module@vger.kernel.org

Acked-by: James Morris <james.l.morris@oracle.com>


-- 
James Morris
<jmorris@namei.org>

^ permalink raw reply

* Re: [PATCH 1/4] Add packet recirculation
From: Simon Horman @ 2013-04-09  7:50 UTC (permalink / raw)
  To: Jesse Gross
  Cc: dev@openvswitch.org, netdev, Ravi K, Isaku Yamahata, Ben Pfaff
In-Reply-To: <CAEP_g=9nj24hZcA-czzuz3xaweATxRxmibuFaqODTSoPmRmHsw@mail.gmail.com>

On Mon, Apr 08, 2013 at 06:46:29PM -0700, Jesse Gross wrote:
> On Sun, Apr 7, 2013 at 11:43 PM, Simon Horman <horms@verge.net.au> wrote:
> > diff --git a/datapath/actions.c b/datapath/actions.c
> > index e9634fe..7b0f022 100644
> > --- a/datapath/actions.c
> > +++ b/datapath/actions.c
> > @@ -617,6 +617,9 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
> >                 case OVS_ACTION_ATTR_SAMPLE:
> >                         err = sample(dp, skb, a);
> >                         break;
> > +
> > +               case OVS_ACTION_ATTR_RECIRCULATE:
> > +                       return 1;
> 
> I think that if we've had a previous output action with the port
> stored in prev_port then this will cause the packet to not actually be
> output.

I'm not so sure.

I see something like this occurring:

1. Iteration of for loop for output action

   switch (nla_type(a)) {
   case OVS_ACTION_ATTR_OUTPUT:
	prev_port = nla_get_u32(a);
	break;
	...
   }

2. Iteration of of for loop for next action, lets say its is recirculate

   i. Output packet

   if (prev_port != -1) {
	do_output(dp, skb_clone(skb, GFP_ATOMIC), prev_port);
	prev_port = -1;
   }

   ii. Return due to recirculate
   switch (nla_type(a)) {
   ...
   case OVS_ACTION_ATTR_RECIRCULATE:
           return 1;
   }


Am I missing something?

> > diff --git a/datapath/datapath.c b/datapath/datapath.c
> > index e8be795..ab39dd7 100644
> > --- a/datapath/datapath.c
> > +++ b/datapath/datapath.c
> >  void ovs_dp_process_received_packet(struct vport *p, struct sk_buff *skb)
> [...]
> > +               if (IS_ERR_OR_NULL(skb)) {
> > +                       break;
> > +               } else if (unlikely(!limit--)) {
> 
> Should this be a predecrement?

I will make it so.

> > +                       kfree_skb(skb);
> 
> Should we log some kind of rate limited warning here?

Sure.

> > +                       return;
> 
> In the first case we use break to exit the loop and here we use
> return.  Both should have the same effect so it might be nice to make
> them the same.
> 
> > @@ -901,6 +913,9 @@ static int validate_and_copy_actions__(const struct nlattr *attr,
> >                         skip_copy = true;
> >                         break;
> >
> > +               case OVS_ACTION_ATTR_RECIRCULATE:
> > +                       break;
> 
> I think we might want to jump out the loop here to better model how
> the actions are actually executed.

Sure, perhaps something like this?

diff --git a/datapath/datapath.c b/datapath/datapath.c
index ab39dd7..721a52c 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -914,7 +914,7 @@ static int validate_and_copy_actions__(const struct nlattr *attr,
 			break;
 
 		case OVS_ACTION_ATTR_RECIRCULATE:
-			break;
+			goto out;
 
 		default:
 			return -EINVAL;
@@ -926,6 +926,7 @@ static int validate_and_copy_actions__(const struct nlattr *attr,
 		}
 	}
 
+out:
 	if (rem > 0)
 		return -EINVAL;
 

> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index e4a2f75..31255f6 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> >  dp_netdev_port_input(struct dp_netdev *dp, struct dp_netdev_port *port,
> >                       struct ofpbuf *packet)
> [...]
> > +        } else {
> > +            dp->n_missed++;
> > +            dp_netdev_output_userspace(dp, packet, DPIF_UC_MISS, &key, NULL);
> > +            recirculate = false;
> > +        }
> > +    } while (recirculate && limit--);
> 
> I have the same question about predecrement here.

I will change this one too.

> > @@ -1163,6 +1190,7 @@ dp_netdev_sample(struct dp_netdev *dp,
> >      const struct nlattr *subactions = NULL;
> >      const struct nlattr *a;
> >      size_t left;
> > +    uint32_t skb_mark;
> 
> I don't think it's right to have a new (and uninitialized) copy of
> skb_mark here.  We should have the same one all the way through, like
> we do in the kernel.

Sure. I will pass it as an argument to dp_netdev_sample()

> > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> > index 47830c1..5129da1 100644
> > --- a/ofproto/ofproto-dpif.c
> > +++ b/ofproto/ofproto-dpif.c
> 
> I'm still working on more detailed comments for this.  However, I'm
> concerned about whether the behavior for revalidation and stats is
> correct.

I am a little concerned about that too.
Perhaps Ben could look over it?

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox