Re: [PATCH 4.9 2/2] tipc: Fix backport of b77413446408fdd256599daf00d5be72b5f3e7c6

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Tue, Jul 27, 2021 at 03:56:50PM -0700, Nathan Chancellor wrote:
> Clang warns:
> 
> net/tipc/link.c:896:23: warning: variable 'hdr' is uninitialized when
> used here [-Wuninitialized]
>         imp = msg_importance(hdr);
>                              ^~~
> net/tipc/link.c:890:22: note: initialize the variable 'hdr' to silence
> this warning
>         struct tipc_msg *hdr;
>                             ^
>                              = NULL
> 1 warning generated.
> 
> The backport of commit b77413446408 ("tipc: fix NULL deref in
> tipc_link_xmit()") to 4.9 as commit 310014f572a5 ("tipc: fix NULL deref
> in tipc_link_xmit()") added the hdr initialization above the
> 
>     if (unlikely(msg_size(hdr) > mtu)) {
> 
> like in the upstream commit; however, in 4.9, that check is below imp's
> first use because commit 365ad353c256 ("tipc: reduce risk of user
> starvation during link congestion") is not present. This results in hdr
> being used uninitialized.
> 
> Fix this by moving hdr's initialization before imp and after the if
> check like the original backport did.
> 
> Cc: Hoang Le <hoang.h.le@dektech.com.au>
> Cc: Jon Maloy <jon.maloy@ericsson.com>
> Cc: Ying Xue <ying.xue@windriver.com>
> Fixes: 310014f572a5 ("tipc: fix NULL deref in tipc_link_xmit()")
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
>  net/tipc/link.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/tipc/link.c b/net/tipc/link.c
> index 06327f78f203..6fc2fa75503d 100644
> --- a/net/tipc/link.c
> +++ b/net/tipc/link.c
> @@ -893,6 +893,7 @@ int tipc_link_xmit(struct tipc_link *l, struct sk_buff_head *list,
>  	if (pkt_cnt <= 0)
>  		return 0;
>  
> +	hdr = buf_msg(skb_peek(list));
>  	imp = msg_importance(hdr);
>  	/* Match msg importance against this and all higher backlog limits: */
>  	if (!skb_queue_empty(backlogq)) {
> @@ -902,7 +903,6 @@ int tipc_link_xmit(struct tipc_link *l, struct sk_buff_head *list,
>  		}
>  	}
>  
> -	hdr = buf_msg(skb_peek(list));
>  	if (unlikely(msg_size(hdr) > mtu)) {
>  		skb_queue_purge(list);
>  		return -EMSGSIZE;
> -- 
> 2.32.0.264.g75ae10bc75
> 

Thanks for these, now both queued up.

greg k-h