[PATCH net] netrom: fix info-leak in nr_write_internal()

[PATCH net] netrom: fix info-leak in nr_write_internal()

[Eric Dumazet]

Simon Kapadia reported the following issue:

<quote>

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:

</quote>

Simon provided a patch that I slightly revised:
Not only we must not use skb_tailroom(), we also do
not want to count NR_NETWORK_LEN twice.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Co-Developed-by: Simon Kapadia <szymon@kapadia.pl>
Signed-off-by: Simon Kapadia <szymon@kapadia.pl>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Tested-by: Simon Kapadia <szymon@kapadia.pl>
---
 net/netrom/nr_subr.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/netrom/nr_subr.c b/net/netrom/nr_subr.c
index 3f99b432ea707e20a9620fb89cdf37d5e4f121e9..e2d2af924cff4a4103e59e04a6efe69c6fcca23e 100644
--- a/net/netrom/nr_subr.c
+++ b/net/netrom/nr_subr.c
@@ -123,7 +123,7 @@ void nr_write_internal(struct sock *sk, int frametype)
 	unsigned char  *dptr;
 	int len, timeout;
 
-	len = NR_NETWORK_LEN + NR_TRANSPORT_LEN;
+	len = NR_TRANSPORT_LEN;
 
 	switch (frametype & 0x0F) {
 	case NR_CONNREQ:
@@ -141,7 +141,8 @@ void nr_write_internal(struct sock *sk, int frametype)
 		return;
 	}
 
-	if ((skb = alloc_skb(len, GFP_ATOMIC)) == NULL)
+	skb = alloc_skb(NR_NETWORK_LEN + len, GFP_ATOMIC);
+	if (!skb)
 		return;
 
 	/*
@@ -149,7 +150,7 @@ void nr_write_internal(struct sock *sk, int frametype)
 	 */
 	skb_reserve(skb, NR_NETWORK_LEN);
 
-	dptr = skb_put(skb, skb_tailroom(skb));
+	dptr = skb_put(skb, len);
 
 	switch (frametype & 0x0F) {
 	case NR_CONNREQ:
-- 
2.40.1.698.g37aff9b760-goog

Re: [PATCH net] netrom: fix info-leak in nr_write_internal()

[Simon Horman]

On Wed, May 24, 2023 at 02:14:56PM +0000, Eric Dumazet wrote:
> Simon Kapadia reported the following issue:
> 
> <quote>
> 
> 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:
> 
> </quote>
> 
> Simon provided a patch that I slightly revised:
> Not only we must not use skb_tailroom(), we also do
> not want to count NR_NETWORK_LEN twice.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Co-Developed-by: Simon Kapadia <szymon@kapadia.pl>
> Signed-off-by: Simon Kapadia <szymon@kapadia.pl>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Tested-by: Simon Kapadia <szymon@kapadia.pl>

Reviewed-by: Simon Horman <simon.horman@corigine.com>

Re: [PATCH net] netrom: fix info-leak in nr_write_internal()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 24 May 2023 14:14:56 +0000 you wrote:
> Simon Kapadia reported the following issue:
> 
> <quote>
> 
> 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.:
> 
> [...]

Here is the summary with links:
  - [net] netrom: fix info-leak in nr_write_internal()
    https://git.kernel.org/netdev/net/c/31642e7089df

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html