linux-kernel - Re: [PATCH v3 1/3] zram: fix crashes due to use of cpu hotplug multistate

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 for Android: free password hash cracker in your pocket

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

Date:   Tue, 22 Jun 2021 08:12:10 -0700
From:   Luis Chamberlain <mcgrof@...nel.org>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     minchan@...nel.org, jeyu@...nel.org, ngupta@...are.org,
        sergey.senozhatsky.work@...il.com, axboe@...nel.dk,
        mbenes@...e.com, jpoimboe@...hat.com, tglx@...utronix.de,
        keescook@...omium.org, jikos@...nel.org, rostedt@...dmis.org,
        peterz@...radead.org, linux-block@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/3] zram: fix crashes due to use of cpu hotplug
 multistate

On Tue, Jun 22, 2021 at 09:39:40AM +0200, Greg KH wrote:
> On Mon, Jun 21, 2021 at 04:30:11PM -0700, Luis Chamberlain wrote:
> > CPU 1                            CPU 2
> > 
> > class_unregister(...);
> 
> Now the sysfs files are removed and invalidated for all devices
> associated with that class.

You're right the disksize_store() would have to happen before.

> > idr_for_each(...);
> > zram_debugfs_destroy();
> >                                 disksize_store(...);
> 
> How will this call into the kobject's store function if
> class_unregister() has already happened?

The disksize_store() would indeed have to happen before.

> > idr_destroy(...);
> > unregister_blkdev(...);
> 
> Ah, it's a block device's store function you are worried about, not the
> class one?

In this case its about any files which can change the cpu compression
streams.

> > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > index cf8deecc39ef..431b60cd85c1 100644
> > --- a/drivers/block/zram/zram_drv.c
> > +++ b/drivers/block/zram/zram_drv.c
> > @@ -44,6 +44,8 @@ static DEFINE_MUTEX(zram_index_mutex);
> >  static int zram_major;
> >  static const char *default_compressor = CONFIG_ZRAM_DEF_COMP;
> >  
> > +bool zram_up;
> 
> static?

Will fix.

> > +
> >  /* Module params (documentation at end) */
> >  static unsigned int num_devices = 1;
> >  /*
> > @@ -1704,6 +1706,7 @@ static void zram_reset_device(struct zram *zram)
> >  	comp = zram->comp;
> >  	disksize = zram->disksize;
> >  	zram->disksize = 0;
> > +	zram->comp = NULL;
> 
> Is this a new change?

It is a sanity change, but indeed, it is separate. I'll let Minchan
decide if he would prefer this to go out as a separate change.

  Luis