Re: [next] unix stream crashes
From: Sedat Dilek
Date: Sat Sep 03 2011 - 09:47:40 EST
On Sat, Sep 3, 2011 at 2:30 PM, Yan, Zheng <yanzheng@xxxxxxxx> wrote:
> The skb can be destructed before the while loop in unix_stream_sendmsg stops.
> please try below patch.
>
> ---
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index e6d9d10..f6d7ed7 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -1577,6 +1577,7 @@ static int unix_stream_sendmsg(struct kiocb
> *kiocb, struct socket *sock,
> Â Â Â Âint sent = 0;
> Â Â Â Âstruct scm_cookie tmp_scm;
> Â Â Â Âbool fds_sent = false;
> + Â Â Â bool scm_ref = true;
> Â Â Â Âint max_level;
>
> Â Â Â Âif (NULL == siocb->scm)
> @@ -1637,12 +1638,19 @@ static int unix_stream_sendmsg(struct kiocb
> *kiocb, struct socket *sock,
> Â Â Â Â Â Â Â Â */
> Â Â Â Â Â Â Â Âsize = min_t(int, size, skb_tailroom(skb));
>
> + Â Â Â Â Â Â Â /*
> + Â Â Â Â Â Â Â Â* pass the scm reference to the skb if a single skb is large
> + Â Â Â Â Â Â Â Â* enough to hold all data.
> + Â Â Â Â Â Â Â Â*/
> + Â Â Â Â Â Â Â if (!fds_sent && sent + size >= len)
> + Â Â Â Â Â Â Â Â Â Â Â scm_ref = false;
>
> - Â Â Â Â Â Â Â /* Only send the fds and no ref to pid in the first buffer */
> - Â Â Â Â Â Â Â err = unix_scm_to_skb(siocb->scm, skb, !fds_sent, fds_sent);
> + Â Â Â Â Â Â Â /* Only send the fds in the first buffer */
> + Â Â Â Â Â Â Â err = unix_scm_to_skb(siocb->scm, skb, !fds_sent,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â fds_sent || scm_ref);
> Â Â Â Â Â Â Â Âif (err < 0) {
> Â Â Â Â Â Â Â Â Â Â Â Âkfree_skb(skb);
> - Â Â Â Â Â Â Â Â Â Â Â goto out;
> + Â Â Â Â Â Â Â Â Â Â Â goto out_err;
> Â Â Â Â Â Â Â Â}
> Â Â Â Â Â Â Â Âmax_level = err + 1;
> Â Â Â Â Â Â Â Âfds_sent = true;
> @@ -1650,7 +1658,7 @@ static int unix_stream_sendmsg(struct kiocb
> *kiocb, struct socket *sock,
> Â Â Â Â Â Â Â Âerr = memcpy_fromiovec(skb_put(skb, size), msg->msg_iov, size);
> Â Â Â Â Â Â Â Âif (err) {
> Â Â Â Â Â Â Â Â Â Â Â Âkfree_skb(skb);
> - Â Â Â Â Â Â Â Â Â Â Â goto out;
> + Â Â Â Â Â Â Â Â Â Â Â goto out_err;
> Â Â Â Â Â Â Â Â}
>
> Â Â Â Â Â Â Â Âunix_state_lock(other);
> @@ -1667,10 +1675,10 @@ static int unix_stream_sendmsg(struct kiocb
> *kiocb, struct socket *sock,
> Â Â Â Â Â Â Â Âsent += size;
> Â Â Â Â}
>
> - Â Â Â if (skb)
> - Â Â Â Â Â Â Â scm_release(siocb->scm);
> - Â Â Â else
> + Â Â Â if (scm_ref)
> Â Â Â Â Â Â Â Âscm_destroy(siocb->scm);
> + Â Â Â else
> + Â Â Â Â Â Â Â scm_release(siocb->scm);
> Â Â Â Âsiocb->scm = NULL;
>
> Â Â Â Âreturn sent;
> @@ -1683,9 +1691,10 @@ pipe_err:
> Â Â Â Â Â Â Â Âsend_sig(SIGPIPE, current, 0);
> Â Â Â Âerr = -EPIPE;
> Âout_err:
> - Â Â Â if (skb == NULL)
> + Â Â Â if (scm_ref)
> Â Â Â Â Â Â Â Âscm_destroy(siocb->scm);
> -out:
> + Â Â Â else
> + Â Â Â Â Â Â Â scm_release(siocb->scm);
> Â Â Â Âsiocb->scm = NULL;
> Â Â Â Âreturn sent ? : err;
> Â}
>
>
I have tested your patch on i386 against:
1. linux-next/patch-v3.1-rc3-next-20110826
2. scm-fix/0001-Revert-Scm-Remove-unnecessary-pid-credential-referen.patch
3. scm-fix-2/scm_send.patch
4. scm-fix-3/0001-Fix-unix-stream-crashes.patch
So the BROKEN scm-send path seems to be fixed, now!
As the patch arrived "malformed" in my mbox I git-am-ed it on top of
linux-next (next-20110826) GIT repository (patch attached).
After confirmation of Valdis (x86_64) and ACK-by Tim, I would
appreciate a proper patch with all Reported-by/Tested-by/S-o-b etc.
In my case I bisected the issue, I recall that there is sth. like
Bisected-by, so feel free to do so.
Doing now a 2nd run with:
1. linux-next/patch-v3.1-rc3-next-20110826
2. scm-fix-3/0001-Fix-unix-stream-crashes.patch
- Sedat -
> On Sat, Sep 3, 2011 at 2:23 PM, Jiri Slaby <jirislaby@xxxxxxxxx> wrote:
>> On 09/03/2011 07:54 AM, Sedat Dilek wrote:
>>>
>>> I saw similiar call-traces with put_cred_rcu() - besides with
>>> kmem_cache_alloc_trace().
>>> My post-it says:
>>> Kernel panic - not syncing: CRED: put_cred_rcu sees f67ac0c0 with usage
>>> -43
>>
>> Hm, Tim, it looks like you put a pid which you did not get?
>>
>> regards,
>> --
>> js
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at Âhttp://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at Âhttp://www.tux.org/lkml/
>>
>
From 54d8a5c590c06f070d9adbfffba0b32246d727e2 Mon Sep 17 00:00:00 2001
From: "Yan, Zheng" <yanzheng@xxxxxxxx>
Date: Sat, 3 Sep 2011 14:30:19 +0200
Subject: [PATCH] Fix unix stream crashes
The skb can be destructed before the while loop in unix_stream_sendmsg stops.
please try below patch.
---
net/unix/af_unix.c | 27 ++++++++++++++++++---------
1 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index e6d9d10..f6d7ed7 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1577,6 +1577,7 @@ static int unix_stream_sendmsg(struct kiocb *kiocb, struct socket *sock,
int sent = 0;
struct scm_cookie tmp_scm;
bool fds_sent = false;
+ bool scm_ref = true;
int max_level;
if (NULL == siocb->scm)
@@ -1637,12 +1638,19 @@ static int unix_stream_sendmsg(struct kiocb *kiocb, struct socket *sock,
*/
size = min_t(int, size, skb_tailroom(skb));
+ /*
+ * pass the scm reference to the skb if a single skb is large
+ * enough to hold all data.
+ */
+ if (!fds_sent && sent + size >= len)
+ scm_ref = false;
- /* Only send the fds and no ref to pid in the first buffer */
- err = unix_scm_to_skb(siocb->scm, skb, !fds_sent, fds_sent);
+ /* Only send the fds in the first buffer */
+ err = unix_scm_to_skb(siocb->scm, skb, !fds_sent,
+ fds_sent || scm_ref);
if (err < 0) {
kfree_skb(skb);
- goto out;
+ goto out_err;
}
max_level = err + 1;
fds_sent = true;
@@ -1650,7 +1658,7 @@ static int unix_stream_sendmsg(struct kiocb *kiocb, struct socket *sock,
err = memcpy_fromiovec(skb_put(skb, size), msg->msg_iov, size);
if (err) {
kfree_skb(skb);
- goto out;
+ goto out_err;
}
unix_state_lock(other);
@@ -1667,10 +1675,10 @@ static int unix_stream_sendmsg(struct kiocb *kiocb, struct socket *sock,
sent += size;
}
- if (skb)
- scm_release(siocb->scm);
- else
+ if (scm_ref)
scm_destroy(siocb->scm);
+ else
+ scm_release(siocb->scm);
siocb->scm = NULL;
return sent;
@@ -1683,9 +1691,10 @@ pipe_err:
send_sig(SIGPIPE, current, 0);
err = -EPIPE;
out_err:
- if (skb == NULL)
+ if (scm_ref)
scm_destroy(siocb->scm);
-out:
+ else
+ scm_release(siocb->scm);
siocb->scm = NULL;
return sent ? : err;
}
--
1.7.6