Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1355774 > unrolled thread

Re: [PATCH 02/18] mm: make vm_mmap killable

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 02/18] mm: make vm_mmap killable Vlastimil Babka <vbabka@suse.cz> - 2016-03-11 11:00 +0100
    Re: [PATCH 02/18] mm: make vm_mmap killable Michal Hocko <mhocko@kernel.org> - 2016-03-11 13:20 +0100
      Re: [PATCH 02/18] mm: make vm_mmap killable Vlastimil Babka <vbabka@suse.cz> - 2016-03-11 13:50 +0100
        Re: [PATCH 02/18] mm: make vm_mmap killable Michal Hocko <mhocko@kernel.org> - 2016-03-11 14:00 +0100

#1355774 — Re: [PATCH 02/18] mm: make vm_mmap killable

On 02/29/2016 02:26 PM, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
>
> All the callers of vm_mmap seem to check for the failure already
> and bail out in one way or another on the error which means that

Hmm I'm not that sure about this one:
   17   1071  fs/binfmt_elf.c <<load_elf_binary>>

Assigns result of vm_mmap() to "error" variable which is never checked. 
Shouldn't __must_check trigger here?

[toc] | [next] | [standalone]

#1355870

On Fri 11-03-16 10:59:30, Vlastimil Babka wrote:
> On 02/29/2016 02:26 PM, Michal Hocko wrote:
> >From: Michal Hocko <mhocko@suse.com>
> >
> >All the callers of vm_mmap seem to check for the failure already
> >and bail out in one way or another on the error which means that
> 
> Hmm I'm not that sure about this one:
>   17   1071  fs/binfmt_elf.c <<load_elf_binary>>
> 
> Assigns result of vm_mmap() to "error" variable which is never checked.

Yes it is not checked but not used either. If the current got killed
then it wouldn't return to the userspace so my understanding is that not
checking this value is not a problem. At least that is my understanding.

> Shouldn't __must_check trigger here?

well, __must_check is a misleading name. It doesn't actually enforce the
value is checked. It just has to be used and an assignment is
sufficient. I was discussing this without our gcc guy and he promissed
to look and try to come up with a different attribute which would
actually work like __must_check.

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1355885

On 03/11/2016 01:12 PM, Michal Hocko wrote:
> On Fri 11-03-16 10:59:30, Vlastimil Babka wrote:
>> On 02/29/2016 02:26 PM, Michal Hocko wrote:
>> >From: Michal Hocko <mhocko@suse.com>
>> >
>> >All the callers of vm_mmap seem to check for the failure already
>> >and bail out in one way or another on the error which means that
>>
>> Hmm I'm not that sure about this one:
>>   17   1071  fs/binfmt_elf.c <<load_elf_binary>>
>>
>> Assigns result of vm_mmap() to "error" variable which is never checked.
>
> Yes it is not checked but not used either. If the current got killed
> then it wouldn't return to the userspace so my understanding is that not
> checking this value is not a problem. At least that is my understanding.

Hmm, that's true. So,

Acked-by: Vlastimil Babka <vbabka@suse.cz>

>> Shouldn't __must_check trigger here?
>
> well, __must_check is a misleading name. It doesn't actually enforce the
> value is checked. It just has to be used and an assignment is
> sufficient. I was discussing this without our gcc guy and he promissed
> to look and try to come up with a different attribute which would
> actually work like __must_check.

OK!

[toc] | [prev] | [next] | [standalone]

#1355892

On Fri 11-03-16 13:43:41, Vlastimil Babka wrote:
> On 03/11/2016 01:12 PM, Michal Hocko wrote:
> >On Fri 11-03-16 10:59:30, Vlastimil Babka wrote:
> >>On 02/29/2016 02:26 PM, Michal Hocko wrote:
> >>>From: Michal Hocko <mhocko@suse.com>
> >>>
> >>>All the callers of vm_mmap seem to check for the failure already
> >>>and bail out in one way or another on the error which means that
> >>
> >>Hmm I'm not that sure about this one:
> >>  17   1071  fs/binfmt_elf.c <<load_elf_binary>>
> >>
> >>Assigns result of vm_mmap() to "error" variable which is never checked.
> >
> >Yes it is not checked but not used either. If the current got killed
> >then it wouldn't return to the userspace so my understanding is that not
> >checking this value is not a problem. At least that is my understanding.
> 
> Hmm, that's true. So,

I have updated the changelog and added the following note:
"
Please note that load_elf_binary is ignoring vm_mmap error for 
current->personality & MMAP_PAGE_ZERO case but that shouldn't be a
problem because the address is not used anywhere and we never return to
the userspace if we got killed.
"
 
> Acked-by: Vlastimil Babka <vbabka@suse.cz>

Thanks!

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web