linux-kernel - Re: [PATCH v2 1/2] md: Fix overflow in is_mddev

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] [day] [month] [year] [list]

Message-ID: <8713b617-db39-113a-377f-f97e123dbf9b@huaweicloud.com>
Date: Tue, 19 Dec 2023 09:39:51 +0800
From: Yu Kuai <yukuai1@...weicloud.com>
To: Song Liu <song@...nel.org>, Yu Kuai <yukuai1@...weicloud.com>
Cc: linan666@...weicloud.com, axboe@...nel.dk, linux-raid@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-block@...r.kernel.org,
 yi.zhang@...wei.com, houtao1@...wei.com, yangerkun@...wei.com,
 "yukuai (C)" <yukuai3@...wei.com>
Subject: Re: [PATCH v2 1/2] md: Fix overflow in is_mddev_idle

Hi,

在 2023/12/19 0:04, Song Liu 写道:
> On Sun, Dec 17, 2023 at 5:39 PM Yu Kuai <yukuai1@...weicloud.com> wrote:
>>
> [...]
>>>
>>> We only use  this for idle or not check, the behavior is OK (I think).
>>> However, this logic is error prone.
>>>
>>> On 64-bit systems, there is a 4-byte hole behind sync_io. I think we can
>>> just use it for atomic64_t so that we don't have to worry about overflow.
>>
>> I'm not sure about this, because other than this ubsan warning, this
>> overflow doesn't have any impact on functionality to me.
> 
> Fixing warnings for zero or low cost is always a good idea. It helps boost
> the signal when UBSAN (and other debug features) detects real issues.
> 
>> If we care about this 'hole', there are lots of holes in gendisk, and
>> can be avoiled, for example, moving 'sync_io' near  to 'node_id'.
> 
> The point was not "let's fill the hole", but "we can use atomic64_t
> without extra memory cost". In general, I don't think we care too
> much about holes in "struct gendisk".
> 
> Does this make sense?

Of course, I don't have strong preference on this. Because our syzkaller
reported lots of UBSAN warnings, hence only fix real issues is how we do
it. For upstream, I'm good at fix this warning with zero or low cost.

Thanks,
Kuai

> 
> Thanks,
> Song
> .
>