Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) client-ip=2620:137:e000::3:4;
Date:   Tue, 28 Nov 2023 09:18:23 -0800
From:   Eric Biggers <ebiggers@kernel.org>
To:     Sergei Shtepa <sergei.shtepa@linux.dev>
Cc:     axboe@kernel.dk, hch@infradead.org, corbet@lwn.net,
        snitzer@kernel.org, mingo@redhat.com, peterz@infradead.org,
        juri.lelli@redhat.com, viro@zeniv.linux.org.uk, brauner@kernel.org,
        linux-block@vger.kernel.org, linux-doc@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        Sergei Shtepa <sergei.shtepa@veeam.com>
Subject: Re: [PATCH v6 11/11] blksnap: prevents using devices with data
 integrity or inline encryption
Message-ID: <20231128171823.GA1148@sol.localdomain>
References: <20231124165933.27580-1-sergei.shtepa@linux.dev>
 <20231124165933.27580-12-sergei.shtepa@linux.dev>
 <20231127224719.GD1463@sol.localdomain>
 <6cabaa42-c366-4928-8294-ad261dae0043@linux.dev>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <6cabaa42-c366-4928-8294-ad261dae0043@linux.dev>
Precedence: bulk

On Tue, Nov 28, 2023 at 12:00:17PM +0100, Sergei Shtepa wrote:
> But I haven't tested the code on a device where hardware inline encryption is
> available. I would be glad if anyone could help with this.
> > 
> > Anyway, this patch is better than ignoring the problem.  It's worth noting,
> > though, that this patch does not prevent blksnap from being set up on a block
> > device on which blk-crypto-fallback is already being used (or will be used).
> > When that happens, I/O will suddenly start failing.  For usability reasons,
> > ideally that would be prevented somehow.
> 
> I didn't observe any failures during testing. It's just that the snapshot
> image shows files with encrypted names and data. Backup in this case is
> useless. Unfortunately, there is no way to detect a blk-crypto-fallback on
> the block device filter level.

Huh, I thought that this patch is supposed to exclude blk-crypto-fallback too.
__submit_bio() calls bio->bi_bdev->bd_filter->ops->submit_bio(bio) before
blk_crypto_bio_prep(), so doesn't your check of ->bi_crypt_context exclude
blk-crypto-fallback?

I think you're right that it might actually be fine to use blksnap with
blk-crypto-fallback, provided that the encryption is done first.  I would like
to see a proper explanation of that, though.  And we still have this patch which
claims that it doesn't work, which is confusing.

- Eric