linux-kernel - Re: [PATCH] ntb_hw_switchtec: Fix shift-out-of-bounds in switchtec_ntb_mw_set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20230817172516.GA321366@bhelgaas>
Date:   Thu, 17 Aug 2023 12:25:16 -0500
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Yajun Deng <yajun.deng@...ux.dev>
Cc:     kurt.schwemmer@...rosemi.com, logang@...tatee.com,
        jdmason@...zu.us, dave.jiang@...el.com, allenbh@...il.com,
        linux-pci@...r.kernel.org, ntb@...ts.linux.dev,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ntb_hw_switchtec: Fix shift-out-of-bounds in
 switchtec_ntb_mw_set_trans

On Wed, Aug 16, 2023 at 04:33:05PM +0800, Yajun Deng wrote:
> There is a kernel API ntb_mw_clear_trans() would pass 0 to both addr and
> size. This would make xlate_pos negative.
> 
> [   23.734156] switchtec switchtec0: MW 0: part 0 addr 0x0000000000000000 size 0x0000000000000000
> [   23.734158] ================================================================================
> [   23.734172] UBSAN: shift-out-of-bounds in drivers/ntb/hw/mscc/ntb_hw_switchtec.c:293:7
> [   23.734418] shift exponent -1 is negative
> 
> Ensuring xlate_pos is a positive or zero before BIT.

I assume Kurt or Logan will apply this and no need to repost for this,
but if you do repost for some reason, the timestamps and separator
lines above are clutter and don't contribute to understanding the
problem.

Also s/Ensuring/Ensure/

> Fixes: 1e2fd202f859 ("ntb_hw_switchtec: Check for alignment of the buffer in mw_set_trans()")
> Signed-off-by: Yajun Deng <yajun.deng@...ux.dev>
> ---
>  drivers/ntb/hw/mscc/ntb_hw_switchtec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/ntb/hw/mscc/ntb_hw_switchtec.c b/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
> index d6bbcc7b5b90..21468d4fef64 100644
> --- a/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
> +++ b/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
> @@ -288,7 +288,7 @@ static int switchtec_ntb_mw_set_trans(struct ntb_dev *ntb, int pidx, int widx,
>  	if (size != 0 && xlate_pos < 12)
>  		return -EINVAL;
>  
> -	if (!IS_ALIGNED(addr, BIT_ULL(xlate_pos))) {
> +	if (xlate_pos >= 0 && !IS_ALIGNED(addr, BIT_ULL(xlate_pos))) {
>  		/*
>  		 * In certain circumstances we can get a buffer that is
>  		 * not aligned to its size. (Most of the time
> -- 
> 2.25.1
>