linux-kernel - Re: [PATCH v6 2/4] block: add blk

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:   Wed, 10 Mar 2021 10:04:40 +0000
From:   Christoph Hellwig <hch@...radead.org>
To:     Sergei Shtepa <sergei.shtepa@...am.com>
Cc:     Christoph Hellwig <hch@...radead.org>,
        "snitzer@...hat.com" <snitzer@...hat.com>,
        "agk@...hat.com" <agk@...hat.com>, "hare@...e.de" <hare@...e.de>,
        "song@...nel.org" <song@...nel.org>,
        "axboe@...nel.dk" <axboe@...nel.dk>,
        "dm-devel@...hat.com" <dm-devel@...hat.com>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-raid@...r.kernel.org" <linux-raid@...r.kernel.org>,
        "linux-api@...r.kernel.org" <linux-api@...r.kernel.org>,
        Pavel Tide <Pavel.TIde@...am.com>
Subject: Re: [PATCH v6 2/4] block: add blk_interposer

On Wed, Mar 10, 2021 at 07:53:13AM +0300, Sergei Shtepa wrote:
> > Please avoid the overly long line.
> > 
> > > +	int ret = 0;
> > > +
> > > +	if (WARN_ON(!interposer))
> > 
> > WARN_ON_ONCE?
> 
> This function should be called quite rarely, and the absence of the interposer
> parameter indicates that the function is being used incorrectly.
> I would like to see this warning every time.

Yes.  Most kernel code would in fact just remove the check entirely
and let the kernel crash to indicate this.  Maybe that is an even
better option for such a grave API usage mistake.

> > > +struct bdev_interposer {
> > > +	ip_submit_bio_t ip_submit_bio;
> > > +	struct block_device *bdev;
> > 
> > Do we need the ip_ prefix here?  Also we probably don't really the
> > the typedef for the function pointer.
> 
> Ok. Maybe submit_bio_hook would be better? or submit_bio_interposer.

Or interpose_bio?