linux-kernel - Re: [PATCH v2 2/2] mtd: spi-nor: expose internal parameters via debugfs

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <f303280eea2e7a4bc2d14a5cd22727b1@walle.cc>
Date:   Fri, 29 Apr 2022 11:44:49 +0200
From:   Michael Walle <michael@...le.cc>
To:     Pratyush Yadav <p.yadav@...com>
Cc:     Tudor Ambarus <tudor.ambarus@...rochip.com>,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        Richard Weinberger <richard@....at>,
        Vignesh Raghavendra <vigneshr@...com>,
        linux-kernel@...r.kernel.org, linux-mtd@...ts.infradead.org
Subject: Re: [PATCH v2 2/2] mtd: spi-nor: expose internal parameters via
 debugfs

Hi,

>> +#define SNOR_F_NAME(name) [ilog2(SNOR_F_##name)] = #name
>> +static const char *const snor_f_names[] = {
>> +	SNOR_F_NAME(HAS_SR_TB),
>> +	SNOR_F_NAME(NO_OP_CHIP_ERASE),
>> +	SNOR_F_NAME(BROKEN_RESET),
>> +	SNOR_F_NAME(4B_OPCODES),
>> +	SNOR_F_NAME(HAS_4BAIT),
>> +	SNOR_F_NAME(HAS_LOCK),
>> +	SNOR_F_NAME(HAS_16BIT_SR),
>> +	SNOR_F_NAME(NO_READ_CR),
>> +	SNOR_F_NAME(HAS_SR_TB_BIT6),
>> +	SNOR_F_NAME(HAS_4BIT_BP),
>> +	SNOR_F_NAME(HAS_SR_BP3_BIT6),
>> +	SNOR_F_NAME(IO_MODE_EN_VOLATILE),
>> +	SNOR_F_NAME(SOFT_RESET),
>> +	SNOR_F_NAME(SWP_IS_VOLATILE),
>> +};
>> +#undef SNOR_F_NAME
> 
> You said you would add a comment here. Changed your mind?

No, it just slipped through.

>> +void spi_nor_debugfs_register(struct spi_nor *nor)
>> +{
>> +	struct dentry *d;
>> +	int ret;
>> +
>> +	/* Create rootdir once. Will never be deleted again. */
>> +	if (!rootdir)
>> +		rootdir = debugfs_create_dir("spi-nor", NULL);
> 
> If I compile SPI NOR as module, I insmod it, rmmod it, and then insmod
> it again, I get:
> 
> 	[  360.623465] spi-nor spi0.0: mt35xu512aba (65536 Kbytes)
> 	[  360.623478] debugfs: Directory 'spi-nor' with parent '/' already 
> present!
> 	[  360.632237] spi-nor spi1.0: mt25qu512a (65536 Kbytes)
> 
> I guess when you remove the module, rootdir also gets destroyed, and
> then gets re-initialized on probing again. I am not familiar enough 
> with
> the debugfs APIs to suggest a fix though.

Thanks for testing. And yes, you are right. I've changed that code
quite a few times for this damn subdirectory. But it seems I didn't
get it right. Usually, it's created in the module_init() but since
we just have an implicit one (and I don't really want to change that),
that's not an option. Some subsystems don't create a subdirectory at
all, which doesn't appeal to me.

I'll first lookup if the directory is already there, if it is, use it,
if not, create it. That should work. FWIW, the mvpp2 network card driver
does it too.

-michael