Skip to contentSkip to conversation

rxrpc: sendmsg: Fix variable overwrite in rxrpc_queue_packet

PATCH Superseded Gustavo A. R. Silva 2 participants 5 messages Nov 27, 2017

Subsystems (3)
Networking Network drivers Rxrpc sockets (AF_RXRPC)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Gustavo A. R. Silva

author
#0 lore raw

PATCH 1/1 rxrpc: sendmsg: Fix variable overwrite in rxrpc_queue_packet

Value assigned to variable resend_at is overwritten before it can be used.

Fix this by removing the value overwrite as it seems that this is a leftover code.

Addresses-Coverity-ID: 1462262

Fixes beb8e5e4f38c ("rxrpc: Express protocol timeouts in terms of RTT")
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 net/rxrpc/sendmsg.c | 1 -
 1 file changed, 1 deletion(-)
net/rxrpc/sendmsg.c +0 -1
index a1c53ac..42060dd 100644
@@ -233,7 +233,6 @@ static void rxrpc_queue_packet(struct rxrpc_sock *rx, struct rxrpc_call *call,
 		if (resend_at < 1)
 			resend_at = 1;
 
-		resend_at = now + rxrpc_resend_timeout;
 		WRITE_ONCE(call->resend_at, resend_at);
 		rxrpc_reduce_call_timer(call, resend_at, now,
 					rxrpc_timer_set_for_send);
Signature · 1 line
2.7.4
maintainer
#1 lore raw
Quoted 4 lines · Gustavo A. R. Silva

Value assigned to variable resend_at is overwritten before it can be used.

Fix this by removing the value overwrite as it seems that this is a leftover code.

NAK. Your fix will actually cause the code to break.

The resend_at value used for the timer must be based on the current time (ie. jiffies in this case), so we can't simply remove that line as the previously calculated resend_at value is a duration, not a time.

What you need to do is to instead modify the line you wanted to remove to add 'now' to the previously-computed value.

David

author
#2 lore raw

Hi David,

Quoting David Howells <[email protected]>:

Quoted 7 lines
Quoted 4 lines · Gustavo A. R. Silva

Value assigned to variable resend_at is overwritten before it can be used.

Fix this by removing the value overwrite as it seems that this is a leftover code.

Quoted 9 lines

NAK. Your fix will actually cause the code to break.

The resend_at value used for the timer must be based on the current time (ie. jiffies in this case), so we can't simply remove that line as the previously calculated resend_at value is a duration, not a time.

What you need to do is to instead modify the line you wanted to remove to add 'now' to the previously-computed value.

You mean something like the following?

net/rxrpc/sendmsg.c +1 -1
@@ -233,7 +233,7 @@ static void rxrpc_queue_packet(struct rxrpc_sock *rx, struct rxrpc_call *call,
                if (resend_at < 1)
                        resend_at = 1;
 
-               resend_at = now + rxrpc_resend_timeout;
+               resend_at += now;
                WRITE_ONCE(call->resend_at, resend_at);
                rxrpc_reduce_call_timer(call, resend_at, now,
                                        rxrpc_timer_set_for_send);

Thanks
--
Gustavo A. R. Silva

maintainer
#3 lore raw

net/rxrpc/sendmsg.c static void rxrpc_queue_packet(struct rxrpc_sock *rx, struct rxrpc_call *call, · line 236

Quoted 2 lines · Gustavo A. R. Silva
net/rxrpc/sendmsg.c +1 -1
-               resend_at = now + rxrpc_resend_timeout;
+               resend_at += now;

Yep. :-)

David

author
#4 lore raw

Quoting David Howells <[email protected]>:

Quoted 7 lines

net/rxrpc/sendmsg.c static void rxrpc_queue_packet(struct rxrpc_sock *rx, struct rxrpc_call *call, · line 236

Quoted 2 lines · Gustavo A. R. Silva
net/rxrpc/sendmsg.c +1 -1
-               resend_at = now + rxrpc_resend_timeout;
+               resend_at += now;

Yep. :-)

Great!

What about this one: https://lkml.org/lkml/2017/11/27/810

applies the same?

Thanks