Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1734692

Re: [PATCH] zram: fix null dereference of handle

Show all headers | View raw

Hi Sergey,

On Tue, Sep 19, 2017 at 02:39:35PM +0900, Sergey Senozhatsky wrote:
> On (09/19/17 11:34), Minchan Kim wrote:
> [..]
> >  static void zram_meta_free(struct zram *zram, u64 disksize)
> >  {
> >  	size_t num_pages = disksize >> PAGE_SHIFT;
> > @@ -876,11 +855,18 @@ static int __zram_bvec_read(struct zram *zram, struct page *page, u32 index,
> >  		zram_slot_unlock(zram, index);
> >  	}
> >  
> > -	if (zram_same_page_read(zram, index, page, 0, PAGE_SIZE))
> > -		return 0;
> > -
> >  	zram_slot_lock(zram, index);
> >  	handle = zram_get_handle(zram, index);
> > +	if (unlikely(!handle || zram_test_flag(zram, index, ZRAM_SAME))) {
> > +		void *mem;
> 
> 
> is this branch really unlikely()? ZRAM_SAME ratio really depends,
> on some setups it can be quite likely, I suspect.

Yub. Let's drop it.

> 
> 
> another question, "!handle  ==  value & ZRAM_SAME"? if so, then why not
> just check for `flags & ZRAM_SAME'? if not then:
> 
> -  for `value & ZRAM_SAME' you fill the page with zram_get_element(zram, index)
>    and return 0. ok.
> 
> -  for !handle.... you also fill the page with zram_get_element(zram, index)
>    and return 0. is this ok? shouldn't !handle return error in this case?

We discussed it before that we shouldn't return error.
Userspace can ask reading unallocated buffer freely.

And in this case, it fills the buffer zero because handle and element is unified.
However, if your concern is readability, I will make it more explict.

> 
> 
> I really suspect that there are some paths that can lead to !handle
> entry, that will not be ZRAM_SAME. e.g. error return from compression
> path.

Could you be more specific?

Thanks for the review, Sergey!

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH] zram: fix null dereference of handle Minchan Kim <minchan@kernel.org> - 2017-09-19 04:40 +0200
  Re: [PATCH] zram: fix null dereference of handle Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-09-19 07:40 +0200
    Re: [PATCH] zram: fix null dereference of handle Minchan Kim <minchan@kernel.org> - 2017-09-19 09:00 +0200
      Re: [PATCH] zram: fix null dereference of handle Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-09-19 12:20 +0200

csiph-web