All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Ingo Molnar <mingo@elte.hu>
Cc: chrisw@sous-sol.org, oleg@redhat.com, roland@redhat.com,
	linux-kernel@vger.kernel.org, viro@ZenIV.linux.org.uk
Subject: Re: [patch 1/2] ptrace, security: rename ptrace_may_access => ptrace_access_check
Date: Mon, 11 May 2009 11:51:22 -0700	[thread overview]
Message-ID: <20090511115122.2cf3e98d.akpm@linux-foundation.org> (raw)
In-Reply-To: <20090511133949.GA28533@elte.hu>

On Mon, 11 May 2009 15:39:49 +0200
Ingo Molnar <mingo@elte.hu> wrote:

> 
> * Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Thu, 7 May 2009 11:49:47 +0200
> > Ingo Molnar <mingo@elte.hu> wrote:
> > 
> > > The ptrace_may_access() methods are named confusingly - some 
> > > variants return a bool, while the security subsystem methods have a 
> > > retval convention.
> > > 
> > > Rename it to ptrace_access_check, to reduce the confusion factor. A 
> > > followup patch eliminates the bool usage.
> > 
> > s/may_access/access_check/ is a poor change.  The new name conveys 
> > less information than the old one.
> > 
> > It's quite clear what the return value from "may_access" means.  
> 
> it isnt clear at all. In fact there's two variants: one that returns 
> 'int' and one that returns 'bool' - the two have inverted values.

Oh.  I was assuming this was a yesno-returning function.

> > It's less clear what the return value from a function called 
> > "access_check" means.
> >
> > Switching to something like ptrace_task_accessible() or 
> > ptrace_may_access_task() would be better.
> > 
> > This happens quite often.  The string "check" in the name of a 
> > predicate function is a red flag.
> 
> I disagree. To repeat the argument i made in this thread, the 'may' 
> suggests/attracts a logical value, i.e. yes or no, or boolean. But 
> that goes against the desire of actual call sites wanting a Linux 
> retval.
> 
> I.e. any function name that can be plain-English answered with: 
> 'yes' or 'no' is a red flag for a retval function.
> 
> You cannot answer ptrace_access_check() with 'yes' or 'no'. You 
> could if it was ptrace_access_ok() or ptrace_may_access.
> 

So what _are_ the semantics of the ptrace_may_access() return value?

<checks the code comments>

<stomps off in a huff>

  reply	other threads:[~2009-05-11 18:56 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-05 22:47 [PATCH 3/3] ptrace: do not use task_lock() for attach Oleg Nesterov
2009-05-06  2:08 ` Roland McGrath
2009-05-06  8:00 ` [RFC PATCH 3/3a] ptrace: add _ptrace_may_access() Ingo Molnar
2009-05-06 20:32   ` Roland McGrath
2009-05-06 20:47     ` Christoph Hellwig
2009-05-06 21:09       ` Roland McGrath
2009-05-07  8:19       ` Ingo Molnar
2009-05-07  8:17     ` Ingo Molnar
2009-05-06 23:53   ` Oleg Nesterov
2009-05-07  0:21     ` Roland McGrath
2009-05-07  6:36       ` Oleg Nesterov
2009-05-07  8:20         ` Ingo Molnar
2009-05-07  8:31           ` Oleg Nesterov
2009-05-07  8:38             ` Ingo Molnar
2009-05-07  8:49               ` [patch] security: rename ptrace_may_access => ptrace_access_check Ingo Molnar
2009-05-07  9:19                 ` Oleg Nesterov
2009-05-07  9:27                   ` Ingo Molnar
2009-05-07  8:57               ` [RFC PATCH 3/3a] ptrace: add _ptrace_may_access() Chris Wright
2009-05-07  9:04                 ` Ingo Molnar
2009-05-07  9:20                   ` Chris Wright
2009-05-07  9:54                     ` James Morris
2009-05-07 10:20                       ` your mail Ingo Molnar
2009-05-07 11:37                         ` security: rename ptrace_may_access => ptrace_access_check James Morris
2009-05-07 14:17                           ` Ingo Molnar
2009-06-23 14:14                           ` Oleg Nesterov
2009-06-23 17:49                             ` Christoph Hellwig
2009-06-23 19:24                               ` [PATCH 0/1] mm_for_maps: simplify, use ptrace_may_access() Oleg Nesterov
2009-06-23 19:25                                 ` [PATCH 1/1] " Oleg Nesterov
2009-06-24  3:06                                   ` Serge E. Hallyn
2009-06-24 14:21                                     ` James Morris
2009-06-24  9:25                                   ` Roland McGrath
2009-06-24 14:37                                     ` Oleg Nesterov
2009-06-24  1:08                             ` security: rename ptrace_may_access => ptrace_access_check James Morris
2009-05-08  3:27                         ` your mail Casey Schaufler
2009-06-24 14:19                       ` security: rename ptrace_may_access => ptrace_access_check James Morris
2009-05-07  9:31                   ` [RFC PATCH 3/3a] ptrace: add _ptrace_may_access() Ingo Molnar
2009-05-07  9:49                     ` [patch 1/2] ptrace, security: rename ptrace_may_access => ptrace_access_check Ingo Molnar
2009-05-07 18:47                       ` Roland McGrath
2009-05-07 19:55                       ` Andrew Morton
2009-05-11 13:39                         ` Ingo Molnar
2009-05-11 18:51                           ` Andrew Morton [this message]
2009-05-15  1:10                           ` Américo Wang
2009-05-15 19:34                             ` Ingo Molnar
2009-05-07  9:50                     ` [patch 2/2] ptrace: turn ptrace_access_check() into a retval function Ingo Molnar
2009-05-07 18:47                       ` Roland McGrath
2009-05-06 22:46 ` [PATCH 3/3] ptrace: do not use task_lock() for attach Chris Wright
2009-05-06 23:13   ` Oleg Nesterov
2009-05-06 23:27     ` Chris Wright
2009-05-06 23:48       ` James Morris
2009-05-07  1:17         ` Roland McGrath
2009-05-08 12:18           ` David Howells

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090511115122.2cf3e98d.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=chrisw@sous-sol.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=oleg@redhat.com \
    --cc=roland@redhat.com \
    --cc=viro@ZenIV.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.