Re: [PATCH] drivers/char/mem: mmap readonly MAP_SHARED-/dev/zero correctly

[David Hildenbrand (Arm)]

> 
> It then becomes a question about how to get readonly memory.
> 
>>
>> We can
>>
>> a) Disallow allocating an anon_vma and fail gracefully. So only a shared
>>    zeropage could ever get mapped there. Might break the s390x
>>    mm_forbids_zeropage(). But given that's only used in hypervisors like QEMU,
>>    unlikely.
> 
> Something like:
> 
> 	/* about to maybe prep anon_vma */
> 
> 	if (!vma_cow_mapping(vma) && vma_test(vma, VMA_MAYSHARE_BIT)) {
> 		/* don't prep anon give zero page */
> 	}
> 
> ?

Something like that, but really just failing directly from anon-vma preparation
in a !vma_cow_mapping(vma), maybe using a special error code.

But the less we have to hack around this special case, the better.

> 
> I think though it's surely the only case (I hope!) where you can possibly be
> both anon (as in missing vm_ops) and !CoW? I hope? :)
> 
> So it feels better to fix it at the source.
> 
> OTOH maybe it's worth special-casing so we don't allocate on read.
> 
> But that brings me to c)...
> 
> 
>>
>> b) Do what Lorenzo proposes. This will allocate real memory. Someone decided to
>>    use MAP_SHARED, for unknown reasons, so I'd assume it's unlikely that
>>    something breaks, but you have a point.
> 
> I would say this patch is the right fix for the moment to fix the assert, and we
> can chase up with other approaches afterwards.

It will get backported, so we better be careful.

IIUC, the real change is only for /dev/zero users that

fd = open("/dev/zero", O_RDONLY);
mmap(fd, MAP_SHARED, PROT_READ);

Which is rather something odd to do indeed.

Letting AI do some digging ... it says there are not known users (I don't trust
it, but I suspect at least there are no prominent users, which makes sense ...).


Getting shared memory when requesting MAP_SHARED does sound like the right thing
to do ...

So let's try this:

Acked-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>

-- 
Cheers,

David