[PATCH stable 4.4] tipc: move premature initialization of stack variables

[PATCH stable 4.4] tipc: move premature initialization of stack variables

[Jon Maloy]

From: Jon Paul Maloy <jon.maloy@ericsson.com>

commit 681a55d71799b575f46fe94121728cf67460d1c3 upstream

(Slightly modified to apply cleanly)

In the function tipc_rcv() we initialize a couple of stack variables
from the message header before that same header has been validated.
In rare cases when the arriving header is non-linear, the validation
function itself may linearize the buffer by calling skb_may_pull(),
while the wrongly initialized stack fields are not updated accordingly.

We fix this in this commit.

Reported-by: Matthew Wong <mwong@sonusnet.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

Conflicts:
	net/tipc/node.c

Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
 net/tipc/node.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/net/tipc/node.c b/net/tipc/node.c
index 2df0b98..6660690 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -1272,18 +1272,20 @@ void tipc_rcv(struct net *net, struct sk_buff *skb, struct tipc_bearer *b)
 {
 	struct sk_buff_head xmitq;
 	struct tipc_node *n;
-	struct tipc_msg *hdr = buf_msg(skb);
-	int usr = msg_user(hdr);
+	struct tipc_msg *hdr;
 	int bearer_id = b->identity;
 	struct tipc_link_entry *le;
-	u16 bc_ack = msg_bcast_ack(hdr);
-	int rc = 0;
+	int usr, rc = 0;
+	u16 bc_ack;
 
 	__skb_queue_head_init(&xmitq);
 
-	/* Ensure message is well-formed */
+	/* Ensure message is well-formed before touching the header */
 	if (unlikely(!tipc_msg_validate(skb)))
 		goto discard;
+	hdr = buf_msg(skb);
+	usr = msg_user(hdr);
+	bc_ack = msg_bcast_ack(hdr);
 
 	/* Handle arrival of discovery or broadcast packet */
 	if (unlikely(msg_non_seq(hdr))) {
-- 
2.1.4

Re: [PATCH stable 4.4] tipc: move premature initialization of stack variables

[Greg KH]

On Thu, Apr 27, 2017 at 10:22:52PM +0200, Jon Maloy wrote:
> From: Jon Paul Maloy <jon.maloy@ericsson.com>
> 
> commit 681a55d71799b575f46fe94121728cf67460d1c3 upstream
> 
> (Slightly modified to apply cleanly)

Belongs below the --- line.

> 
> In the function tipc_rcv() we initialize a couple of stack variables
> from the message header before that same header has been validated.
> In rare cases when the arriving header is non-linear, the validation
> function itself may linearize the buffer by calling skb_may_pull(),
> while the wrongly initialized stack fields are not updated accordingly.
> 
> We fix this in this commit.
> 
> Reported-by: Matthew Wong <mwong@sonusnet.com>
> Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> 
> Conflicts:
> 	net/tipc/node.c

What are these lines for?  Please don't do that, "Conflicts:" lines do
not belong in the changelog as they don't make any sense, right?

thanks,

greg k-h

Re: [PATCH stable 4.4] tipc: move premature initialization of stack variables

[Greg KH]

On Fri, Apr 28, 2017 at 09:45:12AM +0200, Greg KH wrote:
> On Thu, Apr 27, 2017 at 10:22:52PM +0200, Jon Maloy wrote:
> > From: Jon Paul Maloy <jon.maloy@ericsson.com>
> > 
> > commit 681a55d71799b575f46fe94121728cf67460d1c3 upstream
> > 
> > (Slightly modified to apply cleanly)
> 
> Belongs below the --- line.
> 
> > 
> > In the function tipc_rcv() we initialize a couple of stack variables
> > from the message header before that same header has been validated.
> > In rare cases when the arriving header is non-linear, the validation
> > function itself may linearize the buffer by calling skb_may_pull(),
> > while the wrongly initialized stack fields are not updated accordingly.
> > 
> > We fix this in this commit.
> > 
> > Reported-by: Matthew Wong <mwong@sonusnet.com>
> > Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
> > Signed-off-by: David S. Miller <davem@davemloft.net>
> > 
> > Conflicts:
> > 	net/tipc/node.c
> 
> What are these lines for?  Please don't do that, "Conflicts:" lines do
> not belong in the changelog as they don't make any sense, right?

Oh, and please fix this up and resend, I didn't take this patch.

greg k-h