From: Eric Dumazet <edumazet@google.com>
To: Simon Kapadia <szymon@kapadia.pl>
Cc: security@kernel.org, Tom M0LTE <tom@m0lte.uk>,
linux-hams@vger.kernel.org
Subject: Re: Bug in the net/rom kernel module
Date: Tue, 23 May 2023 13:32:35 +0200 [thread overview]
Message-ID: <CANn89iJ=eQhM4AJd9qZjn8JTupTpeGeGNeu=x3RAa5zvB+AMHg@mail.gmail.com> (raw)
In-Reply-To: <CAJE3E7+EZVLRoLohp3Y2529HpKSndFZAyakX-FcbEe9wMBe-Bg@mail.gmail.com>
On Tue, May 23, 2023 at 12:47 PM Simon Kapadia <szymon@kapadia.pl> wrote:
>
> Would be much appreciated if you could send a patch -- happy to have my name to it, just haven't done so in this century so would likely make a stylistic error or something stupid!
>
> Thanks,
>
> -simon
>
> --- linux/net/netrom/nr_subr.c.orig 2023-04-19 19:50:03.080791821 +0100
> +++ linux/net/netrom/nr_subr.c 2023-04-19 19:50:22.410995440 +0100
> @@ -149,7 +149,7 @@ void nr_write_internal(struct sock *sk,
> */
> skb_reserve(skb, NR_NETWORK_LEN);
>
> - dptr = skb_put(skb, skb_tailroom(skb));
> + dptr = skb_put(skb, len);
Note that this patch does not look correct to me.
len includes NR_NETWORK_LEN at this point.
nr_transmit_buffer() will later add:
dptr = skb_push(skb, NR_NETWORK_LEN);
If you look again at your pcap after your patch, I guess we still send
15 extra bytes ?
>
>
>
> switch (frametype & 0x0F) {
> case NR_CONNREQ:
>
> --
> Simon Kapadia, B.A., M.Sc., C.Eng.
> szymon@kapadia.pl | http://kapadia.pl
> http://astrophotos.uk/
>
>
> On Tue, 23 May 2023 at 10:55, Eric Dumazet <edumazet@google.com> wrote:
>>
>>
>>
>> On Tue, May 23, 2023 at 11:47 AM Simon Kapadia <szymon@kapadia.pl> wrote:
>>>
>>> Hello there,
>>>
>>> I sent the attached bug report over a month ago to the maintainer of the AX.25 and NETROM modules as per the current MAINTAINERS file, but I haven't had any response.
>>>
>>> While this apparent bug has been around for a long time (it appears to have been introduced in kernel version 2.0 in 1996), I do consider it a security problem. We leak random bits of memory over the network, including user data; see the attached pcaps and screenshots, showing sending information on internal IP addresses and even an ELF binary out over the network. I can't immediately think of a way in which it could directly be remotely exploitable (eg to read a targeted section of memory) but simply leaking private data by dumping raw memory contents onto the network is, in my opinion, a significant concern.
>>>
>>> While this may seem pretty "niche" (who all uses packet networks and amateur radio nowadays, right?*) -- there is quite a large community of users that run AX25 over IP, including using NETROM, which means that this data may be leaked and routed over the internet.
>>>
>>> Is there some way to "escalate" such an issue with the maintainers of the kernel module?
>>>
>>> Many thanks,
>>>
>>> -simon (M0GZP)
>>>
>>> *actually there is quite a resurgence of packet radio in the UK -- start at http://packet.oarc.uk if interested
>>>
>>> --
>>> Simon Kapadia, B.A., M.Sc., C.Eng.
>>> szymon@kapadia.pl | http://kapadia.pl
>>> http://astrophotos.uk/
>>>
>>> ---------- Forwarded message ---------
>>> From: Simon Kapadia <szymon@kapadia.pl>
>>> Date: Wed, 19 Apr 2023 at 21:21
>>> Subject: Bug in the net/rom kernel module
>>> To: <ralf@linux-mips.org>
>>> Cc: <linux-hams@vger.kernel.org>, <tom@m0lte.uk>, <naylorjs@yahoo.com>
>>>
>>>
>>> Hi there,
>>>
>>> The Online Amateur Radio Community (OARC) has recently been experimenting with building a nationwide packet network in the UK. As part of our experimentation, we have been testing out packet on 300bps HF, and playing with net/rom. For HF packet at this baud rate you really need to make sure that your MTU is relatively low; AX.25 suggests a PACLEN of 60, and a net/rom PACLEN of 40 to go with that. However the Linux net/rom support didn't work with a low PACLEN; the mkiss module would truncate packets if you set the PACLEN below about 200 or so, e.g.:
>>>
>>> Apr 19 14:00:51 radio kernel: [12985.747310] mkiss: ax1: truncating oversized transmit packet!
>>>
>>> This didn't make any sense to me (if the packets are smaller why would they be truncated?) so I started investigating. I looked at the packets using ethereal, and found that many were just huge compared to what I would expect. A simple net/rom connection request packet had the request and then a bunch of what appeared to be random data following it:
>>>
>>>
>>>
>>> Weird eh? But then I noticed this in a disconnect request from a friend (Tom, on copy of this mail):
>>>
>>>
>>>
>>> This is leaking internal data from his system. So somehow, his disconnect request has some random bit of memory included in the packet! I did some further experimentation and found that the data included truly is random. Here for example is a packet shown in my TNC log:
>>>
>>> Apr 18 12:14:17 radio direwolf[8011]: [2L] M0GZP-7>M0LTE-7:(I cmd, n(s)=0, n(r)=0, p=0, pid=0xcf)<0x9a>`<0x8e><0xb4><0xa0>@l<0x9a>`<0x98><0xa8><0x8a>@o<0x0f><0x01><0x01><0x00><0x00><0x01><0x04><0x9a>`<0x8e><0xb4><0xa0>@l<0x9a>`<0x8e><0xb4><0xa0>@lx<0x00><0x00><0x00><0x00><0x00><0x00><0x00><0x00><0x00><0x00><0x00><0x00><0x03><0x00><0x00><0x00><0x19><0x00><0x00><0x00><0x10><0x10>4<0xc3><0x10><0x10>4<0xc3><0x00><0x00><0x00><0x00><0x00><0x00><0x00><0x00><0x00><0x00><0x00><0x00><0x00><0x00><0x00><0x00><0xec>o<0xa2><0xbe><0x00><0x00><0x80><0x00><0xff><0xff><0xff><0xff><0x7f>ELF<0x01><0x01><0x01><0x00><0x00><0x00><0x00><0x00><0x00><0x00><0x00><0x00><0x02><0x00>(<0x00><0x01><0x00><0x00><0x00>0'<0x01><0x00>4<0x00><0x00><0x00><0x08><0xd4><0x00><0x00><0x00><0x04><0x00><0x05>4<0x00><0x20><0x00><0x09><0x00>(<0x00><0x1d><0x00><0x1c><0x00><0x01><0x00><0x00>p<0x00><0xc0><0x00><0x00><0x00><0xc0><0x01><0x00><0x00><0xc0><0x01><0x00><0x08><0x00><0x00><0x00><0x08><0x00><0x00><0x00><0x04><0x00><0x00><0x00><0x04><0x00><0x00><0x00><0x06><0x00><0x00><0x00>4<0x00><0x00><0x00>4<0x00><0x01><0x00>4<0x00><0x0
>>>
>>> 0x7f 0x45 0x4c 0x46 0x02 0x01 0x01 is the header for an ELF program binary, which I'm sending over the wire...
>>>
>>> I spent some time reacquainting myself with kernel development (the last time I did any was the mid 90s), and I think I have worked out what is going on (with copious use of printk of course). Apologies in advance if I've gotten all of this wrong, but I think I'm on solid ground. I narrowed in on the problem when I noticed that this only happens for "internal" packets which are part of the protocol itself. It seems that the problem arises in nr_subr.c in when building up the packet to send:
>>>
>>> void nr_write_internal(struct sock *sk, int frametype)
>>> {
>>> struct nr_sock *nr = nr_sk(sk);
>>> struct sk_buff *skb;
>>> unsigned char *dptr;
>>> int len, timeout;
>>>
>>> len = NR_NETWORK_LEN + NR_TRANSPORT_LEN;
>>>
>>> switch (frametype & 0x0F) {
>>> case NR_CONNREQ:
>>> len += 17;
>>> break;
>>> case NR_CONNACK:
>>> len += (nr->bpqext) ? 2 : 1;
>>> break;
>>> case NR_DISCREQ:
>>> case NR_DISCACK:
>>> case NR_INFOACK:
>>> break;
>>> default:
>>> printk(KERN_ERR "NET/ROM: nr_write_internal - invalid frame type %d\n", frametype);
>>> return;
>>> }
>>>
>>> if ((skb = alloc_skb(len, GFP_ATOMIC)) == NULL)
>>> return;
>>>
>>> /*
>>> * Space for AX.25 and NET/ROM network header
>>> */
>>> skb_reserve(skb, NR_NETWORK_LEN);
>>>
>>> dptr = skb_put(skb, skb_tailroom(skb));
>>>
>>> This code ends up building a packet structure which is too large. The reason for this is the skb_put() function, which is defined in skbuff.h as:
>>>
>>> void *skb_put(struct sk_buff *skb, unsigned int len);
>>>
>>> The skb_tailroom of the struct at this point when it's used in the skb_put is 241 (256 minus the NR_NETWORK_LEN of 15 which was taken by the skb_reserve). By using the tailroom of the struct as the length of the data, we are effectively telling it that the data part of the skbuff should be equal to the tailroom.
>>>
>>> The following patch will make the skb_put use len as the length of the data we want in the packet.
>>>
>>> --- linux/net/netrom/nr_subr.c.orig 2023-04-19 19:50:03.080791821 +0100
>>> +++ linux/net/netrom/nr_subr.c 2023-04-19 19:50:22.410995440 +0100
>>> @@ -149,7 +149,7 @@ void nr_write_internal(struct sock *sk,
>>> */
>>> skb_reserve(skb, NR_NETWORK_LEN);
>>>
>>> - dptr = skb_put(skb, skb_tailroom(skb));
>>> + dptr = skb_put(skb, len);
>>>
>>> switch (frametype & 0x0F) {
>>> case NR_CONNREQ:
>>
>>
>> Hi Simon
>>
>> Can you send an official linux patch to netdev@vger.kernel.org ?
>>
>> We fix similar bugs every day really.
>> With enough time, any hacker can claim a bug is a security one ;)
>>
>> 'The wire' seems limited to L2 domain, which is pretty narrow these days, AX25 is essentially dead.
>>
>> Alternatively, I can cook/send the patch, with your 'Signed-off-by: Simon Kapadia <szymon@kapadia.pl>'
>>
>> Thanks.
>>
>>>
>>>
>>> This appears to work correctly; netrom packets are the right size, you can happily use an AX25 PACLEN of 60 (which for HF means that transmissions are about 2 seconds and not 10), there are no more truncations, and as a bonus no random parts of memory are shared over the network. I've attached two packet captures to this email to support my assertions. bad_cap.pcap shows a number of bad net/rom packets (including data leakage). good_cap.pcap shows a net/rom conversation with the patch applied -- no huge packets, everything working correctly.
>>>
>>> Since this issue does leak random bits of memory over the network, potentially including user data, I considered copying it to security@kernel.org, but since the bug appears to have been around for a long time (looking at history I think it was introduced in kernel version 2.0 in 1996) it's probably not urgent, so I'll leave the decision as to whether there's any need to treat it as a security issue to you :-)
>>>
>>> Many thanks,
>>>
>>> -simon M0GZP
>>>
>>> --
>>> Simon Kapadia, B.A., M.Sc., C.Eng.
>>> szymon@kapadia.pl | http://kapadia.pl
>>> http://astrophotos.uk/
next parent reply other threads:[~2023-05-23 11:32 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CAJE3E7JoJfrnST3dU-vu_=6T_eB5gWVBKbfUSkCMu44gkbD3iQ@mail.gmail.com>
[not found] ` <CAJE3E7LfO5us9MvKZX1MMP8mUTFzO324cxshHTDSw6qkB6kHTw@mail.gmail.com>
[not found] ` <CANn89iKEX9wLYA3aisvVAGKJ2mGr9K35ALZTwHeM7DwEV2+nkg@mail.gmail.com>
[not found] ` <CAJE3E7+EZVLRoLohp3Y2529HpKSndFZAyakX-FcbEe9wMBe-Bg@mail.gmail.com>
2023-05-23 11:32 ` Eric Dumazet [this message]
2023-05-23 11:38 ` Bug in the net/rom kernel module Eric Dumazet
[not found] ` <CAJE3E7+5ox6nx=j+6W4brDnzfKQVMdUiBi0ieFohQDNaWXJxEw@mail.gmail.com>
[not found] ` <CAJE3E7Lb6aQMTGMQ4L63A=EMtYGqpsymW68nWc=nmGnJX-WKHQ@mail.gmail.com>
2023-05-24 12:19 ` Eric Dumazet
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CANn89iJ=eQhM4AJd9qZjn8JTupTpeGeGNeu=x3RAa5zvB+AMHg@mail.gmail.com' \
--to=edumazet@google.com \
--cc=linux-hams@vger.kernel.org \
--cc=security@kernel.org \
--cc=szymon@kapadia.pl \
--cc=tom@m0lte.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).