Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1298470 > unrolled thread
| Started by | Nicolai Stange <nicstange@gmail.com> |
|---|---|
| First post | 2015-12-28 03:50 +0100 |
| Last post | 2015-12-31 12:00 +0100 |
| Articles | 6 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH] net, socket, socket_wq: fix missing initialization of flags Nicolai Stange <nicstange@gmail.com> - 2015-12-28 03:50 +0100
Re: [PATCH] net, socket, socket_wq: fix missing initialization of flags Eric Dumazet <eric.dumazet@gmail.com> - 2015-12-28 18:30 +0100
[PATCH v2] net, socket, socket_wq: fix missing initialization of flags Nicolai Stange <nicstange@gmail.com> - 2015-12-29 13:40 +0100
Re: [PATCH v2] net, socket, socket_wq: fix missing initialization of flags Eric Dumazet <eric.dumazet@gmail.com> - 2015-12-29 20:10 +0100
Re: [PATCH v2] net, socket, socket_wq: fix missing initialization of flags David Miller <davem@davemloft.net> - 2015-12-30 22:40 +0100
Re: [PATCH v2] net, socket, socket_wq: fix missing initialization of flags Nicolai Stange <nicstange@gmail.com> - 2015-12-31 12:00 +0100
| From | Nicolai Stange <nicstange@gmail.com> |
|---|---|
| Date | 2015-12-28 03:50 +0100 |
| Subject | [PATCH] net, socket, socket_wq: fix missing initialization of flags |
| Message-ID | <qKw3k-3Io-753@gated-at.bofh.it> |
Fixes: ceb5d58b2170 ("net: fix sock_wake_async() rcu protection")
Commit ceb5d58b2170 ("net: fix sock_wake_async() rcu protection") from
the current 4.4 release cycle introduces a new flags member in
struct socket_wq and moved SOCKWQ_ASYNC_NOSPACE and SOCKWQ_ASYNC_WAITDATA
from struct socket's flags member into that new place.
Unfortunately, the new flags field is never initialized properly, at least
not for the struct socket_wq instance created in sock_alloc_inode().
One particular issue I encountered because of this is that my GNU Emacs
failed to draw anything on my desktop -- i.e. what I got is a transparent
window, including the title bar. Bisection lead to the commit mentioned
above and further investigation by means of strace told me that Emacs
is indeed speaking to my Xorg through an O_ASYNC AF_UNIX socket. This is
reproducible 100% of times and the fact that properly initializing the
struct socket_wq ->flags fixes the issue leads me to the conclusion that
somehow SOCKWQ_ASYNC_NOSPACE got set in the uninitialized ->flags,
preventing my Emacs from receiving any SIGIO's due to send space becoming
available again and it got stuck.
Make sock_alloc_inode() set the newly created struct socket_wq's ->flags
member to zero.
Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
net/socket.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/socket.c b/net/socket.c
index 29822d6..d730ef9 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -257,6 +257,7 @@ static struct inode *sock_alloc_inode(struct super_block *sb)
}
init_waitqueue_head(&wq->wait);
wq->fasync_list = NULL;
+ wq->flags = 0;
RCU_INIT_POINTER(ei->socket.wq, wq);
ei->socket.state = SS_UNCONNECTED;
--
2.6.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Eric Dumazet <eric.dumazet@gmail.com> |
|---|---|
| Date | 2015-12-28 18:30 +0100 |
| Subject | Re: [PATCH] net, socket, socket_wq: fix missing initialization of flags |
| Message-ID | <qKJMl-5aJ-3@gated-at.bofh.it> |
| In reply to | #1298470 |
On Sun, 2015-12-27 at 21:00 +0100, Nicolai Stange wrote:
> Fixes: ceb5d58b2170 ("net: fix sock_wake_async() rcu protection")
>
> Commit ceb5d58b2170 ("net: fix sock_wake_async() rcu protection") from
> the current 4.4 release cycle introduces a new flags member in
> struct socket_wq and moved SOCKWQ_ASYNC_NOSPACE and SOCKWQ_ASYNC_WAITDATA
> from struct socket's flags member into that new place.
>
> Unfortunately, the new flags field is never initialized properly, at least
> not for the struct socket_wq instance created in sock_alloc_inode().
>
> One particular issue I encountered because of this is that my GNU Emacs
> failed to draw anything on my desktop -- i.e. what I got is a transparent
> window, including the title bar. Bisection lead to the commit mentioned
> above and further investigation by means of strace told me that Emacs
> is indeed speaking to my Xorg through an O_ASYNC AF_UNIX socket. This is
> reproducible 100% of times and the fact that properly initializing the
> struct socket_wq ->flags fixes the issue leads me to the conclusion that
> somehow SOCKWQ_ASYNC_NOSPACE got set in the uninitialized ->flags,
> preventing my Emacs from receiving any SIGIO's due to send space becoming
> available again and it got stuck.
>
> Make sock_alloc_inode() set the newly created struct socket_wq's ->flags
> member to zero.
>
> Signed-off-by: Nicolai Stange <nicstange@gmail.com>
> ---
> net/socket.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/socket.c b/net/socket.c
> index 29822d6..d730ef9 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -257,6 +257,7 @@ static struct inode *sock_alloc_inode(struct super_block *sb)
> }
> init_waitqueue_head(&wq->wait);
> wq->fasync_list = NULL;
> + wq->flags = 0;
> RCU_INIT_POINTER(ei->socket.wq, wq);
>
> ei->socket.state = SS_UNCONNECTED;
Thanks a lot Nicolai for finding this.
I completely overlooked this initial value.
I checked other places where 'struct socket_wq' were allocated and they
look fine.
Acked-by: Eric Dumazet <edumazet@google.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Nicolai Stange <nicstange@gmail.com> |
|---|---|
| Date | 2015-12-29 13:40 +0100 |
| Subject | [PATCH v2] net, socket, socket_wq: fix missing initialization of flags |
| Message-ID | <qL1Jh-xv-13@gated-at.bofh.it> |
| In reply to | #1298713 |
Fixes: ceb5d58b2170 ("net: fix sock_wake_async() rcu protection")
Commit ceb5d58b2170 ("net: fix sock_wake_async() rcu protection") from
the current 4.4 release cycle introduced a new flags member in
struct socket_wq and moved SOCKWQ_ASYNC_NOSPACE and SOCKWQ_ASYNC_WAITDATA
from struct socket's flags member into that new place.
Unfortunately, the new flags field is never initialized properly, at least
not for the struct socket_wq instance created in sock_alloc_inode().
One particular issue I encountered because of this is that my GNU Emacs
failed to draw anything on my desktop -- i.e. what I got is a transparent
window, including the title bar. Bisection lead to the commit mentioned
above and further investigation by means of strace told me that Emacs
is indeed speaking to my Xorg through an O_ASYNC AF_UNIX socket. This is
reproducible 100% of times and the fact that properly initializing the
struct socket_wq ->flags fixes the issue leads me to the conclusion that
somehow SOCKWQ_ASYNC_WAITDATA got set in the uninitialized ->flags,
preventing my Emacs from receiving any SIGIO's due to data becoming
available and it got stuck.
Make sock_alloc_inode() set the newly created struct socket_wq's ->flags
member to zero.
Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
Changes to V1 (only commit message changes):
- Uhm, I misread sock_wake_async(). The meaning of the flags is actually
inverted. My Ghostmacs wasn't waiting for free send space, but some
received data becoming available. This makes indeed more sense and I
replaced "SOCKWQ_ASYNC_NOSPACE" by "SOCKWQ_ASYNC_WAITDATA" in my
speculative explanation above.
- Furthermore I fixed some minor grammar issues.
Sorry for any inconvenience this late-night induced brainfart might
have caused at your side...
net/socket.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/socket.c b/net/socket.c
index 29822d6..d730ef9 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -257,6 +257,7 @@ static struct inode *sock_alloc_inode(struct super_block *sb)
}
init_waitqueue_head(&wq->wait);
wq->fasync_list = NULL;
+ wq->flags = 0;
RCU_INIT_POINTER(ei->socket.wq, wq);
ei->socket.state = SS_UNCONNECTED;
--
2.6.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Eric Dumazet <eric.dumazet@gmail.com> |
|---|---|
| Date | 2015-12-29 20:10 +0100 |
| Subject | Re: [PATCH v2] net, socket, socket_wq: fix missing initialization of flags |
| Message-ID | <qL7OG-4A4-11@gated-at.bofh.it> |
| In reply to | #1298973 |
On Tue, 2015-12-29 at 13:29 +0100, Nicolai Stange wrote:
> Fixes: ceb5d58b2170 ("net: fix sock_wake_async() rcu protection")
> Sorry for any inconvenience this late-night induced brainfart might
> have caused at your side...
Not really, because I read the source code and saw the bug there ;)
Acked-by: Eric Dumazet <edumazet@google.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | David Miller <davem@davemloft.net> |
|---|---|
| Date | 2015-12-30 22:40 +0100 |
| Subject | Re: [PATCH v2] net, socket, socket_wq: fix missing initialization of flags |
| Message-ID | <qLwDo-3xs-7@gated-at.bofh.it> |
| In reply to | #1298973 |
From: Nicolai Stange <nicstange@gmail.com>
Date: Tue, 29 Dec 2015 13:29:55 +0100
> Fixes: ceb5d58b2170 ("net: fix sock_wake_async() rcu protection")
>
> Commit ceb5d58b2170 ("net: fix sock_wake_async() rcu protection") from
> the current 4.4 release cycle introduced a new flags member in
> struct socket_wq and moved SOCKWQ_ASYNC_NOSPACE and SOCKWQ_ASYNC_WAITDATA
> from struct socket's flags member into that new place.
>
> Unfortunately, the new flags field is never initialized properly, at least
> not for the struct socket_wq instance created in sock_alloc_inode().
>
> One particular issue I encountered because of this is that my GNU Emacs
> failed to draw anything on my desktop -- i.e. what I got is a transparent
> window, including the title bar. Bisection lead to the commit mentioned
> above and further investigation by means of strace told me that Emacs
> is indeed speaking to my Xorg through an O_ASYNC AF_UNIX socket. This is
> reproducible 100% of times and the fact that properly initializing the
> struct socket_wq ->flags fixes the issue leads me to the conclusion that
> somehow SOCKWQ_ASYNC_WAITDATA got set in the uninitialized ->flags,
> preventing my Emacs from receiving any SIGIO's due to data becoming
> available and it got stuck.
>
> Make sock_alloc_inode() set the newly created struct socket_wq's ->flags
> member to zero.
>
> Signed-off-by: Nicolai Stange <nicstange@gmail.com>
Applied, but please in the future please put the Fixes: tag right
above the first signoff/ack, like this:
Fixes: ceb5d58b2170 ("net: fix sock_wake_async() rcu protection")
Signed-off-by: Nicolai Stange <nicstange@gmail.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Nicolai Stange <nicstange@gmail.com> |
|---|---|
| Date | 2015-12-31 12:00 +0100 |
| Subject | Re: [PATCH v2] net, socket, socket_wq: fix missing initialization of flags |
| Message-ID | <qLJ7z-3mu-5@gated-at.bofh.it> |
| In reply to | #1299499 |
David Miller <davem@davemloft.net> writes:
> From: Nicolai Stange <nicstange@gmail.com>
> Date: Tue, 29 Dec 2015 13:29:55 +0100
>
>> Fixes: ceb5d58b2170 ("net: fix sock_wake_async() rcu protection")
>>
>> Commit ceb5d58b2170 ("net: fix sock_wake_async() rcu protection") from
>> the current 4.4 release cycle introduced a new flags member in
>> struct socket_wq and moved SOCKWQ_ASYNC_NOSPACE and SOCKWQ_ASYNC_WAITDATA
>> from struct socket's flags member into that new place.
>>
>> Unfortunately, the new flags field is never initialized properly, at least
>> not for the struct socket_wq instance created in sock_alloc_inode().
>>
>> One particular issue I encountered because of this is that my GNU Emacs
>> failed to draw anything on my desktop -- i.e. what I got is a transparent
>> window, including the title bar. Bisection lead to the commit mentioned
>> above and further investigation by means of strace told me that Emacs
>> is indeed speaking to my Xorg through an O_ASYNC AF_UNIX socket. This is
>> reproducible 100% of times and the fact that properly initializing the
>> struct socket_wq ->flags fixes the issue leads me to the conclusion that
>> somehow SOCKWQ_ASYNC_WAITDATA got set in the uninitialized ->flags,
>> preventing my Emacs from receiving any SIGIO's due to data becoming
>> available and it got stuck.
>>
>> Make sock_alloc_inode() set the newly created struct socket_wq's ->flags
>> member to zero.
>>
>> Signed-off-by: Nicolai Stange <nicstange@gmail.com>
>
> Applied, but please in the future please put the Fixes: tag right
> above the first signoff/ack, like this:
>
> Fixes: ceb5d58b2170 ("net: fix sock_wake_async() rcu protection")
> Signed-off-by: Nicolai Stange <nicstange@gmail.com>
Thank you very much!
Regarding the correct position of the "Fixes:" tag: lesson learned.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web