Messages in this thread |  | | | Date | Fri, 2 Mar 2012 16:03:10 +0100 | | From | Oleg Nesterov <> | | Subject | Re: [RFC] c/r: prctl: Add ability to set new mm_struct::exe_file |
| |
On 03/02, Cyrill Gorcunov wrote: > > On Thu, Mar 01, 2012 at 08:41:20PM +0100, Oleg Nesterov wrote: > ... > > > > Still can't understand. I think you need: > > > > file = fget(fd); > > if (!file) > > return -EBADF; > > > > down_write(&mm->mmap_sem); > > if (mm->num_exe_file_vmas) { > > fput(mm->exe_file); > > mm->exe_file = file; > > file = NULL; > > } > > up_write(&mm->mmap_sem); > > > > if (!file) > > return 0; > > > > fput(file); > > return -ESOMETHING; > > > > and that is all. > > This breaks overall logic of num_exe_file_vmas.
Why? In my opinion, your patch breaks the logic ;)
> What the point to have it at all then?
I think it should die. I already suggested to do
- struct file *exe_file; + struct path *exe_path;
and kill this counter, but this is off-topic.
> I mean, > if there several executable sections in elf file, > once loader finish its work we will have > num_exe_file_vmas more than 1.
Yes. And?
> Then the process calls for prctl and replaces > own exe_file (I'm talking about possible scenario > since for our own tool we know that there will be > only one .text section and we're more-less safe > in replacing own exe_file,
confused. I do not see the "num_exe_file_vmas == 1" check in the last version. (yes, I think it is not needed).
OTOH, you should check num_exe_file_vmas != 0, otherwise you break the current logic.
> but this interface > will be available for everyone who has c/r config > entry turned on,
Yes, and thus it should work in any case.
> so I'm trying to find which > negative impact this feature might have,
If you find something negative - please explain and correct me ;)
Your message starts with "This breaks overall logic" without any explanation.
> so once process has replaced own exe_file > to something else the code which depends on > num_exe_file_vmas become broken.
Again, why???
> May not we have a scenario when removed_exe_file_vma > is be called somewhere else later, once this prctl > finished its work? That's what I fear of.
Of course, removed_exe_file_vma() or added_exe_file_vma() can be called after prctl(). And we should keep the current logic: mm->exe_file exists until num_exe_file_vmas != 0.
To simplify, currently we have:
- num_exe_file_vmas is equal to the number of MAP_EXECUTABLE vmas
- (num_exe_file_vmas != 0) <=> (exe_file != NULL)
You should keep this. Or you should change the rules and explain why you are doing this.
Oleg.
|  |