On 2024/12/6 19:53, Lorenzo Stoakes wrote:
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.?
Not familiar with ovl, but we convert from file to realfile, adding cred
to prevent potential problems. maybe Amir could help to confirm it :)
+
+ 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.
Yes, should directly return.
+ }
+
+ 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?
Great, I thin Jinjiang could continue to this work.
