| Message ID | 877fke6tqi.fsf@doppelsaurus.mobileactivedefense.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On 16.12.2015 21:09, Rainer Weikusat wrote: > With b3ca9b02b00704053a38bfe4c31dbbb9c13595d0, the AF_UNIX SOCK_STREAM > receive code was changed from using mutex_lock(&u->readlock) to > mutex_lock_interruptible(&u->readlock) to prevent signals from being > delayed for an indefinite time if a thread sleeping on the mutex > happened to be selected for handling the signal. But this was never a > problem with the stream receive code (as opposed to its datagram > counterpart) as that never went to sleep waiting for new messages with the > mutex held and thus, wouldn't cause secondary readers to block on the > mutex waiting for the sleeping primary reader. As the interruptible > locking makes the code more complicated in exchange for no benefit, > change it back to using mutex_lock. > > Signed-off-by: Rainer Weikusat <rweikusat@mobileactivedefense.com> > --- > > Considering that the datagram receive routine also doesn't go the sleep > with the mutex held anymore, the 37ab4fa7844a044dc21fde45e2a0fc2f3c3b6490 > change to unix_autobind is now similarly purposeless. I wouldn't do this conversion, yet. There is still a deadlock lingering around which should be solved earlier: http://lists.openwall.net/netdev/2015/11/10/4 Unfortunately I haven't found a good way how to solve it, yet. Thanks, Hannes -- 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
Hannes Frederic Sowa <hannes@stressinduktion.org> writes: > On 16.12.2015 21:09, Rainer Weikusat wrote: >> With b3ca9b02b00704053a38bfe4c31dbbb9c13595d0, the AF_UNIX SOCK_STREAM >> receive code was changed from using mutex_lock(&u->readlock) to >> mutex_lock_interruptible(&u->readlock) to prevent signals from being >> delayed for an indefinite time if a thread sleeping on the mutex >> happened to be selected for handling the signal. But this was never a >> problem with the stream receive code (as opposed to its datagram >> counterpart) as that never went to sleep waiting for new messages with the >> mutex held and thus, wouldn't cause secondary readers to block on the >> mutex waiting for the sleeping primary reader. As the interruptible >> locking makes the code more complicated in exchange for no benefit, >> change it back to using mutex_lock. >> >> Signed-off-by: Rainer Weikusat <rweikusat@mobileactivedefense.com> >> --- >> >> Considering that the datagram receive routine also doesn't go the sleep >> with the mutex held anymore, the 37ab4fa7844a044dc21fde45e2a0fc2f3c3b6490 >> change to unix_autobind is now similarly purposeless. > > I wouldn't do this conversion, yet. There is still a deadlock lingering > around which should be solved earlier: > > http://lists.openwall.net/netdev/2015/11/10/4 > > Unfortunately I haven't found a good way how to solve it, yet. Judging from the link, that's not related to the stream receive code. -- 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
On 17.12.2015 16:28, Rainer Weikusat wrote: > Hannes Frederic Sowa <hannes@stressinduktion.org> writes: >> On 16.12.2015 21:09, Rainer Weikusat wrote: >>> With b3ca9b02b00704053a38bfe4c31dbbb9c13595d0, the AF_UNIX SOCK_STREAM >>> receive code was changed from using mutex_lock(&u->readlock) to >>> mutex_lock_interruptible(&u->readlock) to prevent signals from being >>> delayed for an indefinite time if a thread sleeping on the mutex >>> happened to be selected for handling the signal. But this was never a >>> problem with the stream receive code (as opposed to its datagram >>> counterpart) as that never went to sleep waiting for new messages with the >>> mutex held and thus, wouldn't cause secondary readers to block on the >>> mutex waiting for the sleeping primary reader. As the interruptible >>> locking makes the code more complicated in exchange for no benefit, >>> change it back to using mutex_lock. >>> >>> Signed-off-by: Rainer Weikusat <rweikusat@mobileactivedefense.com> >>> --- >>> >>> Considering that the datagram receive routine also doesn't go the sleep >>> with the mutex held anymore, the 37ab4fa7844a044dc21fde45e2a0fc2f3c3b6490 >>> change to unix_autobind is now similarly purposeless. >> >> I wouldn't do this conversion, yet. There is still a deadlock lingering >> around which should be solved earlier: >> >> http://lists.openwall.net/netdev/2015/11/10/4 >> >> Unfortunately I haven't found a good way how to solve it, yet. > > Judging from the link, that's not related to the stream receive code. > No, but to commit 37ab4fa7844a044dc21fde45e2a0fc2f3c3b6490 where the mutexes of unix_bind and unix_autobind got changed. The unix_stream_read_generic conversion is fine. Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org> Thanks, Hannes -- 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
From: Rainer Weikusat <rweikusat@mobileactivedefense.com> Date: Wed, 16 Dec 2015 20:09:25 +0000 > With b3ca9b02b00704053a38bfe4c31dbbb9c13595d0, the AF_UNIX SOCK_STREAM > receive code was changed from using mutex_lock(&u->readlock) to > mutex_lock_interruptible(&u->readlock) to prevent signals from being > delayed for an indefinite time if a thread sleeping on the mutex > happened to be selected for handling the signal. But this was never a > problem with the stream receive code (as opposed to its datagram > counterpart) as that never went to sleep waiting for new messages with the > mutex held and thus, wouldn't cause secondary readers to block on the > mutex waiting for the sleeping primary reader. As the interruptible > locking makes the code more complicated in exchange for no benefit, > change it back to using mutex_lock. > > Signed-off-by: Rainer Weikusat <rweikusat@mobileactivedefense.com> Applied, thanks Rainer. -- 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
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c index 1c3c1f3..b1314c0 100644 --- a/net/unix/af_unix.c +++ b/net/unix/af_unix.c @@ -2263,14 +2263,7 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state) /* Lock the socket to prevent queue disordering * while sleeps in memcpy_tomsg */ - err = mutex_lock_interruptible(&u->readlock); - if (unlikely(err)) { - /* recvmsg() in non blocking mode is supposed to return -EAGAIN - * sk_rcvtimeo is not honored by mutex_lock_interruptible() - */ - err = noblock ? -EAGAIN : -ERESTARTSYS; - goto out; - } + mutex_lock(&u->readlock); if (flags & MSG_PEEK) skip = sk_peek_offset(sk, flags); @@ -2314,12 +2307,12 @@ again: timeo = unix_stream_data_wait(sk, timeo, last, last_len); - if (signal_pending(current) || - mutex_lock_interruptible(&u->readlock)) { + if (signal_pending(current)) { err = sock_intr_errno(timeo); goto out; } + mutex_lock(&u->readlock); continue; unlock: unix_state_unlock(sk);
With b3ca9b02b00704053a38bfe4c31dbbb9c13595d0, the AF_UNIX SOCK_STREAM receive code was changed from using mutex_lock(&u->readlock) to mutex_lock_interruptible(&u->readlock) to prevent signals from being delayed for an indefinite time if a thread sleeping on the mutex happened to be selected for handling the signal. But this was never a problem with the stream receive code (as opposed to its datagram counterpart) as that never went to sleep waiting for new messages with the mutex held and thus, wouldn't cause secondary readers to block on the mutex waiting for the sleeping primary reader. As the interruptible locking makes the code more complicated in exchange for no benefit, change it back to using mutex_lock. Signed-off-by: Rainer Weikusat <rweikusat@mobileactivedefense.com> --- Considering that the datagram receive routine also doesn't go the sleep with the mutex held anymore, the 37ab4fa7844a044dc21fde45e2a0fc2f3c3b6490 change to unix_autobind is now similarly purposeless. -- 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