Re: [PATCH 02/46] fork: take mmap_lock before copying mm_struct

[David Hildenbrand (Arm)]

On 10/28/26 22:11, Barry Song wrote:
> On Wed, Oct 28, 2026 at 8:55 PM Pedro Falcato <pfalcato@xxxxxxx> wrote:
>>
>> On Wed, Oct 28, 2026 at 03:48:09AM +0800, Barry Song wrote:
>>> On Wed, Oct 28, 2026 at 3:07 AM David Hildenbrand (Arm)
>>> <david@xxxxxxxxxx> wrote:
>>>
>>> Regarding the race with memcpy(mm, oldmm, sizeof(*mm)), we
>>> still have one [1] even after this patch.
>>>
>>> That is, copying mm_struct can happen concurrently with
>>> lru_gen_del_mm(). However, this race doesn't seem harmful.
>>> I might be missing something here though.
>>
>> I don't think it's harmful as well. mm_init() reinitializes those fields.
>>
>> Though I hate that memcpy. I think that ideally we copy by member and
>> data_race() benign races, like what is done in vm_area_init_from().
> 
> Yes. I was suggesting something like using data_race() to
> silence the warning[1], but applying it to memcpy() also
> seems a bit too drastic.
> 
> Another option is to copy members one by one, skipping those
> that don't need to be copied. However, that would make the
> code quite lengthy, and the copying might also be slower?

The compiler might do its best to improve it, but I don't think this is the
direction we want.

We'd have to figure out

> 
>>
>> Anyway, KCSAN reports aren't the most serious thing in the world, so this can
>> (should?) be cleaned up later.
> 
> Yep, I don't think there are any blockers for this patch. I had
> actually given my Reviewed-by tag before raising this race.
> 
> David discussed races between memcpy() of mm_struct
> and other operations. I recalled another related case, so I
> posted it here. Hopefully, I'm not just making noise.
Not at all. I think that needs a proper investigation as well ... and I am still
u clear how it would be valid to access the mm struct without the mmap lock (per
my patch) that way.

-- 
Cheers,

David