[TIPC] Properly mask header fields

[TIPC] Properly mask header fields

[Max Krasnyansky]

TIPC code is a bit inconsistent in masking out upper bits of various
message fields when packing them into the headers. For the most part
things seem to be ok but we happened to hit a corner case in our labs
when broadcast counter reached certain value (don't remember exact
details) and was messing up status bits in the header. At which point
the link was busted and required a reset to bring it back up.
It's much safer to apply proper mask in the function that does the
actual packing rather than doing it all over the place, and missing a
few ;-).

Signed-off-by: Max Krasnyansky <maxk@qualcomm.com>
---
 net/tipc/msg.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/tipc/msg.h b/net/tipc/msg.h
index 6699aaf..4e681c8 100644
--- a/net/tipc/msg.h
+++ b/net/tipc/msg.h
@@ -72,7 +72,7 @@ static inline void msg_set_bits(struct tipc_msg *m, u32 w,
 				u32 pos, u32 mask, u32 val)
 {
 	u32 word = msg_word(m,w) & ~(mask << pos);
-	msg_set_word(m, w, (word |= (val << pos)));
+	msg_set_word(m, w, (word |= ((val & mask) << pos)));
 }
 
 /* 
-- 
1.4.4.2

[TIPC] Missing null check in the socket code.

[Max Krasnyansky]

Fixes an oops in the non-blocking mode.

Signed-off-by: Max Krasnyansky <maxk@qualcomm.com>
---
 net/tipc/socket.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 2a6a5a6..767f791 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -862,6 +862,10 @@ restart:
 	/* Get access to first message in receive queue */
 
 	buf = skb_peek(&sock->sk->sk_receive_queue);
+	if (NULL == buf) {
+		res = -EAGAIN;
+		goto exit;
+	}
 	msg = buf_msg(buf);
 	sz = msg_data_sz(msg);
 	err = msg_errcode(msg);
-- 
1.4.4.2

Re: [TIPC] Missing null check in the socket code.

[Max Krasnyansky]

Max Krasnyansky wrote:
> Fixes an oops in the non-blocking mode.
> 
> Signed-off-by: Max Krasnyansky <maxk@qualcomm.com>
> ---
>  net/tipc/socket.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> index 2a6a5a6..767f791 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -862,6 +862,10 @@ restart:
>  	/* Get access to first message in receive queue */
>  
>  	buf = skb_peek(&sock->sk->sk_receive_queue);
> +	if (NULL == buf) {
> +		res = -EAGAIN;
> +		goto exit;
> +	}
>  	msg = buf_msg(buf);
>  	sz = msg_data_sz(msg);
>  	err = msg_errcode(msg);

Just a reminder. Please apply.

Thanx
Max

RE: [TIPC] Missing null check in the socket code.

[Jon Maloy (QB/EMC)]

Acknowledged. We'll deliver it along with 
some other patches ASAP.

///jon


-----Original Message-----
From: Max Krasnyansky [mailto:maxk@qualcomm.com]
Sent: Tue 2/20/2007 1:48 PM
To: davem@davemloft.net
Cc: netdev@vger.kernel.org; Jon Maloy (QB/EMC); allan.stephens@windriver.com
Subject: Re: [TIPC] Missing null check in the socket code.
 
Max Krasnyansky wrote:
> Fixes an oops in the non-blocking mode.
> 
> Signed-off-by: Max Krasnyansky <maxk@qualcomm.com>
> ---
>  net/tipc/socket.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> index 2a6a5a6..767f791 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -862,6 +862,10 @@ restart:
>  	/* Get access to first message in receive queue */
>  
>  	buf = skb_peek(&sock->sk->sk_receive_queue);
> +	if (NULL == buf) {
> +		res = -EAGAIN;
> +		goto exit;
> +	}
>  	msg = buf_msg(buf);
>  	sz = msg_data_sz(msg);
>  	err = msg_errcode(msg);

Just a reminder. Please apply.

Thanx
Max

Re: [TIPC] Missing null check in the socket code.

[Max Krasnyansky]

Hi Jon,

> Acknowledged. We'll deliver it along with 
> some other patches ASAP.
Thanx

Did you get a chance to look at my other patch for the header packing ?

Max

Re: [TIPC] Properly mask header fields

[Max Krasnyansky]

Dave, Alan, Jon,

Max Krasnyansky wrote:
> TIPC code is a bit inconsistent in masking out upper bits of various
> message fields when packing them into the headers. For the most part
> things seem to be ok but we happened to hit a corner case in our labs
> when broadcast counter reached certain value (don't remember exact
> details) and was messing up status bits in the header. At which point
> the link was busted and required a reset to bring it back up.
> It's much safer to apply proper mask in the function that does the
> actual packing rather than doing it all over the place, and missing a
> few ;-).
> 
> Signed-off-by: Max Krasnyansky <maxk@qualcomm.com>
> ---
>  net/tipc/msg.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/tipc/msg.h b/net/tipc/msg.h
> index 6699aaf..4e681c8 100644
> --- a/net/tipc/msg.h
> +++ b/net/tipc/msg.h
> @@ -72,7 +72,7 @@ static inline void msg_set_bits(struct tipc_msg *m, u32 w,
>  				u32 pos, u32 mask, u32 val)
>  {
>  	u32 word = msg_word(m,w) & ~(mask << pos);
> -	msg_set_word(m, w, (word |= (val << pos)));
> +	msg_set_word(m, w, (word |= ((val & mask) << pos)));
>  }
>  
>  /* 

Just making sure that this does not get lost in the blizzard of other patches :).
Please apply.

Thanx
Max