Received-SPF: pass (google.com: domain of linux-kernel+bounces-173899-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <212b02a8-f5f0-4433-a726-1639dda61790@linux.microsoft.com>
Date: Wed, 8 May 2024 15:30:14 -0700
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v18 12/21] dm: add finalize hook to target_type
To: Mikulas Patocka <mpatocka@redhat.com>
Cc: corbet@lwn.net, zohar@linux.ibm.com, jmorris@namei.org, serge@hallyn.com,
 tytso@mit.edu, ebiggers@kernel.org, axboe@kernel.dk, agk@redhat.com,
 snitzer@kernel.org, eparis@redhat.com, paul@paul-moore.com,
 linux-doc@vger.kernel.org, linux-integrity@vger.kernel.org,
 linux-security-module@vger.kernel.org, fsverity@lists.linux.dev,
 linux-block@vger.kernel.org, dm-devel@lists.linux.dev,
 audit@vger.kernel.org, linux-kernel@vger.kernel.org
References: <1714775551-22384-1-git-send-email-wufan@linux.microsoft.com>
 <1714775551-22384-13-git-send-email-wufan@linux.microsoft.com>
 <aa767961-5e3-2ceb-1a1e-ff66a8eed649@redhat.com>
Content-Language: en-CA
From: Fan Wu <wufan@linux.microsoft.com>
In-Reply-To: <aa767961-5e3-2ceb-1a1e-ff66a8eed649@redhat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit


On 5/8/2024 10:17 AM, Mikulas Patocka wrote:
> 
> 
> On Fri, 3 May 2024, Fan Wu wrote:
> 
>> This patch adds a target finalize hook.
>>
>> The hook is triggered just before activating an inactive table of a
>> mapped device. If it returns an error the __bind get cancelled.
>>
>> The dm-verity target will use this hook to attach the dm-verity's
>> roothash metadata to the block_device struct of the mapped device.
>>
>> Signed-off-by: Fan Wu <wufan@linux.microsoft.com>
> 
> Hi
> 
> Why not use the preresume callback?
> 
> Is there some reason why do we need a new callback instead of using the
> existing one?
> 
> Mikulas
Thanks for the suggestion.

Mike suggested the new finalize() callback. I think the reason for not 
using the preresume() callback is that there are multiple points that 
can fail before activating an inactive table of a mapped device which 
can potentially lead to inconsistent state.

In our specific case, we are trying to associate dm-verity's roothash 
metadata with the block_device struct of the mapped device inside the 
callback.

If we use the preresume() callback for the work and an error occurs 
between the callback and the table activation, this leave the 
block_device struct in an inconsistent state.

This is because now the block device contains the roothash metadata of 
it's inactive table due to the preresume() callback, but the activation 
failed so the mapped device is still using the old table.

The new finalize() callback guarantees that the callback happens just 
before the table activation, thus avoiding the inconsistency.

-Fan