Re: [PATCH v2 02/10] zram: avoid invalid memory access in zram_exit()

[Jiang Liu]

On Wed 05 Jun 2013 02:04:42 PM CST, Minchan Kim wrote:
> On Wed, Jun 05, 2013 at 12:06:00AM +0800, Jiang Liu wrote:
>> Memory for zram->disk object may have already been freed after returning
>> from destroy_device(zram), then it's unsafe for zram_reset_device(zram)
>> to access zram->disk again.
>>
>> We can't solve this bug by flipping the order of destroy_device(zram)
>> and zram_reset_device(zram), that will cause deadlock issues to the
>> zram sysfs handler.
>
> What kinds of deadlock happen?
> Could you elaborate it more?
>
Hi Minchan,
      I will try my best to explain the situation.
1) if we change the order as:
                zram_reset_device(zram);
                destroy_device(zram);
zram->meta could be rebuilt by disksize_store() just between
zram_reset_device(zram) and destroy_device(zram) because all sysfs
entries are still available, which then cause memory leak.

2) If we change the code as:
        down_write(&zram->init_lock);
        __zram_reset_device(zram);
        destroy_device(zram);
        up_write(&zram->init_lock);
Then it will cause a typical deadlock as:
Thread1:
1) acquire init_lock
2) destroy_device(zram);
2.a)sysfs_remove_group()
2.b) wait for all sysfs files to be closed and released.

Thread2:
1) echo xxm > disksize
2) open sysfs file and call disksize_store()
3) disksize_store() tries to acquire zram->init_lock

Then deadlock.

Regards!
Gerry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/