netdev - Re: sendmsg bug

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHk-=wic8ind8nY5fea+otfkmjBuMwgiXY6idbtrXZcig3yDaA@mail.gmail.com>
Date:   Fri, 4 Mar 2022 13:13:09 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     1031265646 <1031265646@...com>,
        "David S. Miller" <davem@...emloft.net>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        David Ahern <dsahern@...nel.org>,
        Jakub Kicinski <kuba@...nel.org>
Cc:     Netdev <netdev@...r.kernel.org>
Subject: Re: sendmsg bug

[ Appending the full original email - converted to plain-text - below.
Note that the "comments i marked red" are no longer red, since the
html has been stripped out ]

Dear 1031265646,
 please don't email me in private about bugs, much better to send them
to the right person and mailing list (see scripts/get_maintainer.pl in
the kernel sources).

The bug seems real, although mostly harmless. If ip_make_skb() returns
NULL, it's true that udp_sendmsg() will return a misleading success
value since 'err' will be 0 and it will return the length of the
packet that wasn't actually ever created or sent.

UDP being a lossy protocol, probably nobody has cared, since
"successful send" doesn't mean "successful receive" anyway.

I'm not sure what the right error should be for this case, and whether
it should be fixed inside ip_make_skb() ("always return a proper
ERR_PTR") or what.. Or whether it should just be left alone as a
"packet dropped early" thing.

I'll leave that to the network people added to the participants.

            Linus

On Thu, Mar 3, 2022 at 10:23 PM 1031265646 <1031265646@...com> wrote:
>
> hi，
>
> in file udp.c, a function named udp_sendmsg has a code like this:
>
> /* Lockless fast path for the non-corking case. */
> if (!corkreq) {
> skb = ip_make_skb(sk, fl4, getfrag, msg, ulen,
>  sizeof(struct udphdr), &ipc, &rt,
>  msg->msg_flags);
> err = PTR_ERR(skb);
>
>         //here IS_ERR_OR_NULL is expected, instead of !IS_ERR_OR_NULL.
> if (!IS_ERR_OR_NULL(skb))
> err = udp_send_skb(skb, fl4);
> goto out;
> }
>
> but function ip_make_skb may return a null, then err will be set to 0;and out like this:
>
> out:
> ip_rt_put(rt);
> if (free)
> kfree(ipc.opt);
> if (!err)
> return len;  // return a positive value
>
> the ip_make_skb failed means the send operation failed. but a positive value is returnd here. finnally, users regard the operation was success, but actually it failed in kernel.
>
>
> the comments i marked red is the right way i think. which i think is a bug.
>
>
> 1031265646
> 1031265646@...com
> 签名由网易邮箱大师定制