| Message ID | 20230816083305.1426718-1-yajun.deng@linux.dev |
|---|---|
| State | New |
| Headers | show |
| Series | ntb_hw_switchtec: Fix shift-out-of-bounds in switchtec_ntb_mw_set_trans | expand |
On 2023-08-16 02:33, 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. > > Fixes: 1e2fd202f859 ("ntb_hw_switchtec: Check for alignment of the buffer in mw_set_trans()") > Signed-off-by: Yajun Deng <yajun.deng@linux.dev> That makes sense. Thanks! Reviewed-by: Logan Gunthorpe <logang@deltatee.com> Logan
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@linux.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 >
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
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. Fixes: 1e2fd202f859 ("ntb_hw_switchtec: Check for alignment of the buffer in mw_set_trans()") Signed-off-by: Yajun Deng <yajun.deng@linux.dev> --- drivers/ntb/hw/mscc/ntb_hw_switchtec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)