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] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 13 Oct 2017 22:25:18 -0400 (EDT)
From:   Nicolas Pitre <nicolas.pitre@...aro.org>
To:     Al Viro <viro@...IV.linux.org.uk>
cc:     Christoph Hellwig <hch@...radead.org>,
        linux-fsdevel@...r.kernel.org, linux-mm@...ck.org,
        linux-embedded@...r.kernel.org, linux-kernel@...r.kernel.org,
        Chris Brandt <Chris.Brandt@...esas.com>
Subject: Re: [PATCH v6 1/4] cramfs: direct memory access support

On Sat, 14 Oct 2017, Al Viro wrote:

> On Fri, Oct 13, 2017 at 04:09:23PM -0400, Nicolas Pitre wrote:
> > On Fri, 13 Oct 2017, Al Viro wrote:
> > 
> > > OK...  I wonder if it should simply define stubs for kill_mtd_super(),
> > > mtd_unpoint() and kill_block_super() in !CONFIG_MTD and !CONFIG_BLOCK
> > > cases.  mount_mtd() and mount_bdev() as well - e.g.  mount_bdev()
> > > returning ERR_PTR(-ENODEV) and kill_block_super() being simply BUG()
> > > in !CONFIG_BLOCK case.  Then cramfs_kill_sb() would be
> > > 	if (sb->s_mtd) {
> > > 		if (sbi->mtd_point_size)
> > > 			mtd_unpoint(sb->s_mtd, 0, sbi->mtd_point_size);
> > > 		kill_mtd_super(sb);
> > > 	} else {
> > > 		kill_block_super(sb);
> > > 	}
> > > 	kfree(sbi);
> > 
> > Well... Stubs have to be named differently or they conflict with 
> > existing declarations. At that point that makes for more lines of code 
> > compared to the current patch and the naming indirection makes it less 
> > obvious when reading the code. Alternatively I could add those stubs in 
> > the corresponding header files and #ifdef the existing declarations 
> > away. That might look somewhat less cluttered in the main code but it 
> > also hides what is actually going on and left me unconvinced. And I'm 
> > not sure this is worth it in the end given this is not a common 
> > occurrence in the kernel either.
> 
> What I mean is this (completely untested) for CONFIG_BLOCK side of things,
> with something similar for CONFIG_MTD one:
> 
> Provide definitions of mount_bdev/kill_block_super() in case !CONFIG_BLOCK

Yes, that's what I thought you meant, which corresponds to the second 
part of my comment above. And as I said I'm not convinced this hiding of 
kernel config effects is better for understanding what is actually going 
on locally, and my own preference is how things are right now.

But if you confirm you really want things that other way then I'll 
oblige and repost.


Nicolas

Powered by blists - more mailing lists