[PATCH 4.9 1/2] iommu/amd: Fix backport of 140456f994195b568ecd7fc2287a34eadffef3ca

[PATCH 4.9 1/2] iommu/amd: Fix backport of 140456f994195b568ecd7fc2287a34eadffef3ca

[Nathan Chancellor]

Clang warns:

drivers/iommu/amd_iommu.c:1335:6: warning: variable 'flags' is used
uninitialized whenever 'if' condition is true
[-Wsometimes-uninitialized]
        if (!pte)
            ^~~~
drivers/iommu/amd_iommu.c:1352:40: note: uninitialized use occurs here
        spin_unlock_irqrestore(&domain->lock, flags);
                                              ^~~~~
drivers/iommu/amd_iommu.c:1335:2: note: remove the 'if' if its condition
is always false
        if (!pte)
        ^~~~~~~~~
drivers/iommu/amd_iommu.c:1331:21: note: initialize the variable 'flags'
to silence this warning
        unsigned long flags;
                           ^
                            = 0
1 warning generated.

The backport of commit 140456f99419 ("iommu/amd: Fix sleeping in atomic
in increase_address_space()") to 4.9 as commit 1d648460d7c5 ("iommu/amd:
Fix sleeping in atomic in increase_address_space()") failed to keep the
"return false", which in 4.9 needs to be a regular "return" due to a
lack of commit f15d9a992f90 ("iommu/amd: Remove domain->updated").

This resolves the warning and matches the 4.14-4.19 backport.

Cc: Andrey Ryabinin <arbn@yandex-team.com>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Will Deacon <will@kernel.org>
Fixes: 1d648460d7c5 ("iommu/amd: Fix sleeping in atomic in increase_address_space()")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 drivers/iommu/amd_iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 8377bd388d67..14e9b06829d5 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -1333,7 +1333,7 @@ static void increase_address_space(struct protection_domain *domain,
 
 	pte = (void *)get_zeroed_page(gfp);
 	if (!pte)
-		goto out;
+		return;
 
 	spin_lock_irqsave(&domain->lock, flags);
 

base-commit: 0db822f6dee813f746ed196fc561945eee4cd4b9
-- 
2.32.0.264.g75ae10bc75

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

[Nathan Chancellor]

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

Re: [PATCH 4.9 1/2] iommu/amd: Fix backport of 140456f994195b568ecd7fc2287a34eadffef3ca

[Joerg Roedel]

On Tue, Jul 27, 2021 at 03:56:49PM -0700, Nathan Chancellor wrote:
> Clang warns:
> 
> drivers/iommu/amd_iommu.c:1335:6: warning: variable 'flags' is used
> uninitialized whenever 'if' condition is true
> [-Wsometimes-uninitialized]
>         if (!pte)
>             ^~~~
> drivers/iommu/amd_iommu.c:1352:40: note: uninitialized use occurs here
>         spin_unlock_irqrestore(&domain->lock, flags);
>                                               ^~~~~
> drivers/iommu/amd_iommu.c:1335:2: note: remove the 'if' if its condition
> is always false
>         if (!pte)
>         ^~~~~~~~~
> drivers/iommu/amd_iommu.c:1331:21: note: initialize the variable 'flags'
> to silence this warning
>         unsigned long flags;
>                            ^
>                             = 0
> 1 warning generated.
> 
> The backport of commit 140456f99419 ("iommu/amd: Fix sleeping in atomic
> in increase_address_space()") to 4.9 as commit 1d648460d7c5 ("iommu/amd:
> Fix sleeping in atomic in increase_address_space()") failed to keep the
> "return false", which in 4.9 needs to be a regular "return" due to a
> lack of commit f15d9a992f90 ("iommu/amd: Remove domain->updated").
> 
> This resolves the warning and matches the 4.14-4.19 backport.
> 
> Cc: Andrey Ryabinin <arbn@yandex-team.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Will Deacon <will@kernel.org>
> Fixes: 1d648460d7c5 ("iommu/amd: Fix sleeping in atomic in increase_address_space()")
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Acked-by: Joerg Roedel <jroedel@suse.de>

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

[Greg Kroah-Hartman]

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