On Mon, Dec 14, 2020 at 1:49 PM Song Liu <songliubraving@fb.com> wrote:
>
> Here is the root cause of this issue.
>
> We miss a cast to sector_t in raid5_run(). The fix is:
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 39343479ac2a9..ca0b29ac9d9a8 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -7662,7 +7662,7 @@ static int raid5_run(struct mddev *mddev)
>         }
>
>         /* device size must be a multiple of chunk size */
> -       mddev->dev_sectors &= ~(mddev->chunk_sectors - 1);
> +       mddev->dev_sectors &= ~((sector_t)mddev->chunk_sectors - 1);
>         mddev->resync_max_sectors = mddev->dev_sectors;
>
>         if (mddev->degraded > dirty_parity_disks &&

Ok, so this made me go "Hmm, this might be a pattern to look out for".
It's zero-extending a binary 'not', which means that the 'not' only
did low bits, and the zero-extended bits weren't set.

That is probably fine in many situations, but it also does smell like
a problem case. It's one reason why the kernel uses signed types for
some fundamental constants - look at PAGE_SIZE for example. Exactly
because ~(PAGE_SIZE-1) needs to set all the high bits.

Anyway, since I have nothing better to do during the merge window
(hah!) I tried to see if I can come up with a sparse check for this
situation.

Here is my very quick hack to sparse, which I'm just throwing over the
fence to Luc and others to look at (because I still have a _lot_ of
pulls to go through), but it does actually flag the problem in 5.10:

   drivers/md/raid5.c:7665:56: warning: zero-extending a negation -
upper bits not negated

although I'm not entirely sure this won't cause way too many other
warnings for things that are very valid.

So it looks like the warning will be too noisy to be actually useful.

But because it _does_ flag that thing, and because I'm too busy to see
if it might be useful with some more work, I'll just post it here and
see if somebody wants to play with it.

            Linus