On Fri, Dec 06, 2024 at 06:45:11PM +0800, Kefeng Wang wrote:
> So maybe use mm_get_unmapped_area() instead of __get_unmapped_area(),
> something like below,
>
> +static unsigned long ovl_get_unmapped_area(struct file *file,
> + unsigned long addr, unsigned long len, unsigned long pgoff,
> + unsigned long flags)
> +{
> + struct file *realfile;
> + const struct cred *old_cred;
> +
> + realfile = ovl_real_file(file);
> + if (IS_ERR(realfile))
> + return PTR_ERR(realfile);
> +
> + if (realfile->f_op->get_unmapped_area) {
> + unsigned long ret;
> +
> + old_cred = ovl_override_creds(file_inode(file)->i_sb);
> + ret = realfile->f_op->get_unmapped_area(realfile, addr, len,
> + pgoff, flags);
> + ovl_revert_creds(old_cred);
Credentials stuff necessary now you're not having security_mmap_addr()
called etc.?
> +
> + if (ret)
> + return ret;
Surely you'd unconditionally return in this case? I don't think there's any case
where you'd want to fall through.
> + }
> +
> + return mm_get_unmapped_area(current->mm, file, addr, len, pgoff,
> flags);
> +}
>
> Correct me If I'm wrong.
>
> Thanks.
>
I mean this doesn't export anything we don't want exported so this is fine
from that perspective :)
And I guess in principle this is OK in that __get_unmapped_area() would be
invoked on the overlay file, will do the required arch_mmap_check(), then
will invoke your overlay handler.
I did think of suggesting invoking the f_op directly, but it feels icky
vs. just supporting large folios.
But actually... hm I realise I overlooked the fact that underlying _files_
will always provide a large folio-aware handler.
I'm guessing you can't use overlayfs somehow with a MAP_ANON | MAP_SHARED
mapping or similar, thinking of:
if (file) {
...
} else if (flags & MAP_SHARED) {
/*
* mmap_region() will call shmem_zero_setup() to create a file,
* so use shmem's get_unmapped_area in case it can be huge.
*/
get_area = shmem_get_unmapped_area;
}
But surely actually any case that works with overlayfs will have a file and
so... yeah.
Hm, I actually think this should work.
Can you make sure to do some pretty thorough testing on this just to make
sure you're not hitting on any weirdness?
