Re: [PATCH 1/3] zram: fix operator precedence to get offset

[Minchan Kim]

Hi Sergey,

On Fri, Apr 14, 2017 at 02:07:47PM +0900, Sergey Senozhatsky wrote:
> Hello,
> 
> On (04/13/17 09:17), Minchan Kim wrote:
> [..]
> > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > index 9e2199060040..83c38a123242 100644
> > --- a/drivers/block/zram/zram_drv.c
> > +++ b/drivers/block/zram/zram_drv.c
> > @@ -930,7 +930,7 @@ static int zram_rw_page(struct block_device *bdev, sector_t sector,
> >  	}
> >  
> >  	index = sector >> SECTORS_PER_PAGE_SHIFT;
> > -	offset = sector & (SECTORS_PER_PAGE - 1) << SECTOR_SHIFT;
> > +	offset = (sector & (SECTORS_PER_PAGE - 1)) << SECTOR_SHIFT;
> 
> sorry, can it actually produce different results?

I got your point. Actually, offset was wrong but rw_page is called
with PAGE_SIZE io while that offset is related to only partial io
(non-PAGEE size io). IOW, although the wrong offset it is never used
in functions.

To find subtle corruption in ppc64, I added some debug code to
catch up wrong buffer overflow and found it with other bugs but
didn't prove the specific case is valid case or not. Good catch, Sergey!

However, it should be *fixed* to prevent confusion in future but surely,
no need to go to the stable. I will send reply to Greg to prevent merging
it to *stable* when he send review asking to merge.

And next week I will send another fix which *maybe* removes code to get the
offset in zram_rw_page.

Thanks.