Linux-Raid Archives on lore.kernel.org
 help / color / Atom feed
* [PATCH] md: change bitmap offset verification in write_sb_page
@ 2021-01-05 15:06 Jakub Radtke
  2021-01-27  7:52 ` Song Liu
  0 siblings, 1 reply; 3+ messages in thread
From: Jakub Radtke @ 2021-01-05 15:06 UTC (permalink / raw)
  To: song; +Cc: linux-raid

From: Jakub Radtke <jakub.radtke@intel.com>

Removes the code that is correct only for the native metadata.
Write-intent bitmap support for the other metadata formats is blocked.

rdev->sb_start is used in the calculations.
The sb_start is only set and used for native metadata format, and
the bitmap offset check will always fail if it is not set.

In the case of external metadata, the bitmap can be placed in various
places e.g. like the PPL between two volumes (the boundary checks are
performed on the sysfs level and in the mdadm).

Signed-off-by: Jakub Radtke <jakub.radtke@linux.intel.com>
---
 drivers/md/md-bitmap.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 200c5d0f08bf..a78b15df4d82 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -236,14 +236,6 @@ static int write_sb_page(struct bitmap *bitmap, struct page *page, int wait)
 		 */
 		if (mddev->external) {
 			/* Bitmap could be anywhere. */
-			if (rdev->sb_start + offset + (page->index
-						       * (PAGE_SIZE/512))
-			    > rdev->data_offset
-			    &&
-			    rdev->sb_start + offset
-			    < (rdev->data_offset + mddev->dev_sectors
-			     + (PAGE_SIZE/512)))
-				goto bad_alignment;
 		} else if (offset < 0) {
 			/* DATA  BITMAP METADATA  */
 			if (offset
-- 
2.17.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] md: change bitmap offset verification in write_sb_page
  2021-01-05 15:06 [PATCH] md: change bitmap offset verification in write_sb_page Jakub Radtke
@ 2021-01-27  7:52 ` Song Liu
  2021-01-28 13:11   ` Radtke, Jakub
  0 siblings, 1 reply; 3+ messages in thread
From: Song Liu @ 2021-01-27  7:52 UTC (permalink / raw)
  To: Jakub Radtke; +Cc: linux-raid

On Tue, Jan 5, 2021 at 7:06 AM Jakub Radtke
<jakub.radtke@linux.intel.com> wrote:
>
> From: Jakub Radtke <jakub.radtke@intel.com>
>
> Removes the code that is correct only for the native metadata.
> Write-intent bitmap support for the other metadata formats is blocked.
>
> rdev->sb_start is used in the calculations.
> The sb_start is only set and used for native metadata format, and
> the bitmap offset check will always fail if it is not set.

Can we use different logic for native and other metadata, so that we can
keep the check for native metadata? Maybe we can use the combination
of mddev->major_version, mddev->minor_version, and rdev->sb_start?

Thanks,
Song

>
> In the case of external metadata, the bitmap can be placed in various
> places e.g. like the PPL between two volumes (the boundary checks are
> performed on the sysfs level and in the mdadm).
>
> Signed-off-by: Jakub Radtke <jakub.radtke@linux.intel.com>
> ---
>  drivers/md/md-bitmap.c | 8 --------
>  1 file changed, 8 deletions(-)
>
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index 200c5d0f08bf..a78b15df4d82 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -236,14 +236,6 @@ static int write_sb_page(struct bitmap *bitmap, struct page *page, int wait)
>                  */
>                 if (mddev->external) {
>                         /* Bitmap could be anywhere. */
> -                       if (rdev->sb_start + offset + (page->index
> -                                                      * (PAGE_SIZE/512))
> -                           > rdev->data_offset
> -                           &&
> -                           rdev->sb_start + offset
> -                           < (rdev->data_offset + mddev->dev_sectors
> -                            + (PAGE_SIZE/512)))
> -                               goto bad_alignment;
>                 } else if (offset < 0) {
>                         /* DATA  BITMAP METADATA  */
>                         if (offset
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] md: change bitmap offset verification in write_sb_page
  2021-01-27  7:52 ` Song Liu
@ 2021-01-28 13:11   ` Radtke, Jakub
  0 siblings, 0 replies; 3+ messages in thread
From: Radtke, Jakub @ 2021-01-28 13:11 UTC (permalink / raw)
  To: Song Liu; +Cc: linux-raid

Please ignore the patch.
I see that the calculation is working for the other metadata formats too.
The problem was related to my environment, and I misinterpret the 
calculation in write_sb_page, which correctly reports the error.
> On Tue, Jan 5, 2021 at 7:06 AM Jakub Radtke
> <jakub.radtke@linux.intel.com> wrote:
>> From: Jakub Radtke <jakub.radtke@intel.com>
>>
>> Removes the code that is correct only for the native metadata.
>> Write-intent bitmap support for the other metadata formats is blocked.
>>
>> rdev->sb_start is used in the calculations.
>> The sb_start is only set and used for native metadata format, and
>> the bitmap offset check will always fail if it is not set.
> Can we use different logic for native and other metadata, so that we can
> keep the check for native metadata? Maybe we can use the combination
> of mddev->major_version, mddev->minor_version, and rdev->sb_start?
>
> Thanks,
> Song
>
>> In the case of external metadata, the bitmap can be placed in various
>> places e.g. like the PPL between two volumes (the boundary checks are
>> performed on the sysfs level and in the mdadm).
>>
>> Signed-off-by: Jakub Radtke <jakub.radtke@linux.intel.com>
>> ---
>>   drivers/md/md-bitmap.c | 8 --------
>>   1 file changed, 8 deletions(-)
>>
>> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
>> index 200c5d0f08bf..a78b15df4d82 100644
>> --- a/drivers/md/md-bitmap.c
>> +++ b/drivers/md/md-bitmap.c
>> @@ -236,14 +236,6 @@ static int write_sb_page(struct bitmap *bitmap, struct page *page, int wait)
>>                   */
>>                  if (mddev->external) {
>>                          /* Bitmap could be anywhere. */
>> -                       if (rdev->sb_start + offset + (page->index
>> -                                                      * (PAGE_SIZE/512))
>> -                           > rdev->data_offset
>> -                           &&
>> -                           rdev->sb_start + offset
>> -                           < (rdev->data_offset + mddev->dev_sectors
>> -                            + (PAGE_SIZE/512)))
>> -                               goto bad_alignment;
>>                  } else if (offset < 0) {
>>                          /* DATA  BITMAP METADATA  */
>>                          if (offset
>> --
>> 2.17.1
>>


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-05 15:06 [PATCH] md: change bitmap offset verification in write_sb_page Jakub Radtke
2021-01-27  7:52 ` Song Liu
2021-01-28 13:11   ` Radtke, Jakub

Linux-Raid Archives on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-raid/0 linux-raid/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-raid linux-raid/ https://lore.kernel.org/linux-raid \
		linux-raid@vger.kernel.org
	public-inbox-index linux-raid

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-raid


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git