diff mbox series

[net-next] rxrpc: uninitialized variable in rxrpc_send_ack_packet()

Message ID Y3XmQsOFwTHUBSLU@kili (mailing list archive)
State Accepted
Commit 38461894838bbbebab54cbd5a5459cc8d1b6dd9b
Delegated to: Netdev Maintainers
Headers show
Series [net-next] rxrpc: uninitialized variable in rxrpc_send_ack_packet() | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning WARNING: From:/Signed-off-by: email address mismatch: 'From: Dan Carpenter <error27@gmail.com>' != 'Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>'
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Dan Carpenter Nov. 17, 2022, 7:44 a.m. UTC
The "pkt" was supposed to have been deleted in a previous patch.  It
leads to an uninitialized variable bug.

Fixes: 72f0c6fb0579 ("rxrpc: Allocate ACK records at proposal and queue for transmission")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Applies to net-next.

 net/rxrpc/output.c | 2 --
 1 file changed, 2 deletions(-)

Comments

David Howells Nov. 17, 2022, 9:44 a.m. UTC | #1
Dan Carpenter <error27@gmail.com> wrote:

> The "pkt" was supposed to have been deleted in a previous patch.  It
> leads to an uninitialized variable bug.

Weird.  I don't get a compiler warning and the kernel doesn't crash, despite
transmitting millions of acks.

If I disassemble the built code, I see:

   0xffffffff81b09e89 <+723>:   xor    %edi,%edi
   0xffffffff81b09e8b <+725>:   call   0xffffffff811c0bc1 <kfree>

I'm not sure why it's sticking 0 in EDI, though.

David
Dan Carpenter Nov. 17, 2022, 10:34 a.m. UTC | #2
On Thu, Nov 17, 2022 at 09:44:24AM +0000, David Howells wrote:
> Dan Carpenter <error27@gmail.com> wrote:
> 
> > The "pkt" was supposed to have been deleted in a previous patch.  It
> > leads to an uninitialized variable bug.
> 
> Weird.  I don't get a compiler warning and the kernel doesn't crash, despite
> transmitting millions of acks.
> 
> If I disassemble the built code, I see:
> 
>    0xffffffff81b09e89 <+723>:   xor    %edi,%edi
>    0xffffffff81b09e8b <+725>:   call   0xffffffff811c0bc1 <kfree>
> 
> I'm not sure why it's sticking 0 in EDI, though.

We disabled GCC's check for uninitialized variables.  It could be that
you have the .config to automatically zero out stack variables.

CONFIG_CC_HAS_AUTO_VAR_INIT_PATTERN=y
CONFIG_CC_HAS_AUTO_VAR_INIT_ZERO_BARE=y
CONFIG_CC_HAS_AUTO_VAR_INIT_ZERO=y

regards,
dan carpenter
David Howells Nov. 17, 2022, 11:59 a.m. UTC | #3
Dan Carpenter <error27@gmail.com> wrote:

> We disabled GCC's check for uninitialized variables.  It could be that
> you have the .config to automatically zero out stack variables.
> 
> CONFIG_CC_HAS_AUTO_VAR_INIT_PATTERN=y
> CONFIG_CC_HAS_AUTO_VAR_INIT_ZERO_BARE=y
> CONFIG_CC_HAS_AUTO_VAR_INIT_ZERO=y

Ah.  Is there a way to reenable that?

David
David Howells Nov. 17, 2022, 12:04 p.m. UTC | #4
Dan Carpenter <error27@gmail.com> wrote:

> The "pkt" was supposed to have been deleted in a previous patch.  It
> leads to an uninitialized variable bug.
> 
> Fixes: 72f0c6fb0579 ("rxrpc: Allocate ACK records at proposal and queue for transmission")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Acked-by: David Howells <dhowells@redhat.com>
Dan Carpenter Nov. 17, 2022, 1:35 p.m. UTC | #5
On Thu, Nov 17, 2022 at 11:59:32AM +0000, David Howells wrote:
> Dan Carpenter <error27@gmail.com> wrote:
> 
> > We disabled GCC's check for uninitialized variables.  It could be that
> > you have the .config to automatically zero out stack variables.
> > 
> > CONFIG_CC_HAS_AUTO_VAR_INIT_PATTERN=y
> > CONFIG_CC_HAS_AUTO_VAR_INIT_ZERO_BARE=y
> > CONFIG_CC_HAS_AUTO_VAR_INIT_ZERO=y
> 
> Ah.  Is there a way to reenable that?

make W=2 will do it, but W=2 sucks...

regards,
dan carpenter
patchwork-bot+netdevbpf@kernel.org Nov. 18, 2022, 12:10 p.m. UTC | #6
Hello:

This patch was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Thu, 17 Nov 2022 10:44:02 +0300 you wrote:
> The "pkt" was supposed to have been deleted in a previous patch.  It
> leads to an uninitialized variable bug.
> 
> Fixes: 72f0c6fb0579 ("rxrpc: Allocate ACK records at proposal and queue for transmission")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Applies to net-next.
> 
> [...]

Here is the summary with links:
  - [net-next] rxrpc: uninitialized variable in rxrpc_send_ack_packet()
    https://git.kernel.org/netdev/net-next/c/38461894838b

You are awesome, thank you!
diff mbox series

Patch

diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c
index 46432e70a16b..04f945e042ab 100644
--- a/net/rxrpc/output.c
+++ b/net/rxrpc/output.c
@@ -202,7 +202,6 @@  static void rxrpc_cancel_rtt_probe(struct rxrpc_call *call,
 static int rxrpc_send_ack_packet(struct rxrpc_local *local, struct rxrpc_txbuf *txb)
 {
 	struct rxrpc_connection *conn;
-	struct rxrpc_ack_buffer *pkt;
 	struct rxrpc_call *call = txb->call;
 	struct msghdr msg;
 	struct kvec iov[1];
@@ -270,7 +269,6 @@  static int rxrpc_send_ack_packet(struct rxrpc_local *local, struct rxrpc_txbuf *
 		rxrpc_set_keepalive(call);
 	}
 
-	kfree(pkt);
 	return ret;
 }