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

[Sergey Senozhatsky]

Hello,

On (04/15/17 00:33), Minchan Kim wrote:
> On Fri, Apr 14, 2017 at 02:07:47PM +0900, Sergey Senozhatsky wrote:
> > 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.

cool. thanks!

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

sounds interesting!

	-ss