bugfindutils - Bugs: bug #38474, Unintended (?) behaviour change of...

 
 

bug #38474: Unintended (?) behaviour change of -perm +mode predicate

Submitter:  Matthijs Kooijman <matthijs>
Submitted:  Wed 06 Mar 2013 08:50:37 AM UTC
   
 
Category:  find Severity:  3 - Normal
Item Group:  Wrong result Status:  None
Privacy:  Public Assigned to:  jay
Originator Name:  Open/Closed:  Open
Release:  4.5.11 Fixed Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 24 Apr 2013 08:03:30 AM UTC, comment #12: 


> In the case of -perm, there is no existing file mode.


Sure, but POSIX says that find should treat the existing file mode as 0 here.  For example, POSIX says that "find /tmp -perm +rw" finds all files under /tmp that are read-write to everyone and that have no other permissions. Admittedly "+rw" means the same thing as "=rw" here, and "=rw" is clearer; but the point is that there's good precedent for saying that 'find -perm' should mimic chmod's syntax, even when the syntax is obscure.

Paul Eggert <eggert>
Wed 24 Apr 2013 07:13:13 AM UTC, comment #11: 

I've applied Paul's updated patch.   However, I think I agree with Eric on the interpretation of the mode argument.

I propose not to reinstante -perm +MODE with different semantics in the future.

The fact that this went wrong in the first place underlines the fact that more regression test cases are needed for -perm.

I don't find the argument from compatibility with chmod very convincing since the mode argument to chmod is understood to describe a modification to the mode of an existing file.  In the case of -perm, there is no existing file mode.

Hence a description of how the mode should be interpreted that makes perfect sense for chmod could still be confusing for find -perm.

This puts me in the uncomfortable position of wondering if mode_change is really the best basis for find -perm; I'm not sure this is really the intended use case for that function.   But my first point above probably applies to gnulib too; if I wanted mode_compile to reject mode strings of the form +MODE I should probably have contributed a gnulib test case which enforced that.

Anyway I'm marking this bug as not-fixed because better tests are needed (in findutils, at least).

James Youngman <jay>
Group administrator
Mon 22 Apr 2013 10:24:42 PM UTC, comment #10: 

Thanks for catching that problem with 'havekind'.  A patch is attached.

I don't think the code should reject -perm /+0111, or -+0111 for that matter.   'chmod +0111 foo' has a well-defined meaning with GNU chmod, and it's nicer if 'find' is compatible with 'chmod'.  At some point in the future I would like to remove the special check that prohibits 'find -perm +0111', and allow it to work as well, compatibly with GNU chmod.  Now's not a good time to do that, as people may be expecting the no-longer-supported syntax, but after another few years we should be able to enable 'find -perm +0111' with the new meaning.

Checking explicitly for octal digits helps make the code a bit clearer to me, but it's no big deal.  (On architectures where numbers from 0 through 7 can be represented as immediate bits, it might be a tiny bit faster to check for octal digits than to check for decimal digits, but again it's no big deal...)

(file #27931)

Paul Eggert <eggert>
Mon 22 Apr 2013 10:03:08 PM UTC, comment #9: 

Paul's patch, as modified in commit 9040c5d, is not quite right.

The bool havekind variable is now useless; it is set to true by all branches of the switch statement.  It can safely be deleted, along with a later switch statement when havekind is still false.

The code doesn't reject -perm /+0111.  These lines:
  if (NULL == change
      || (perm_expr[0] == '+' && '0' <= perm_expr[1] && perm_expr[1] < '8'))
should probably instead be:
  if (NULL == change
      || (perm_expr[mode_start] == '+' && c_isdigit(perm_expr[mode_start + 1]))

(Note that although c_isdigit will allow '8' and '9', those bogus digits will have already caused the 'NULL == change' branch to be true; so it is a reasonable simplification).

Eric Blake <ericb>
Group administrator
Mon 22 Apr 2013 09:26:30 PM UTC, comment #8: 

I applied the patch in a slightly modified form, mainly to acommodate some changes I'd already made to find.texi.  I also modified the patch to update the manpage as well.   Updated patch is both attached and pushed.

(file #27930)

James Youngman <jay>
Group administrator
Sun 21 Apr 2013 07:38:18 AM UTC, comment #7: 

Let's follow Eric's mild preference and make it an error.  I'm attaching a proposed patch.

(file #27916)

Paul Eggert <eggert>
Sat 20 Apr 2013 05:04:37 PM UTC, comment #6: 


> POSIX only calls out symbolic mode, not numeric mode


Well, POSIX also calls out numeric mode:

-perm [−]onum
 If the <hyphen> is omitted, the primary shall evaluate as true when the file mode
bits exactly match the value of the octal number onum (see the description of the
octal mode in chmod). Otherwise, if onum is prefixed by a <hyphen>, the primary
shall evaluate as true if at least all of the bits specified in onum are set. In both
cases, the behavior is unspecified when onum exceeds 07777.

But there is no mention of parsing a '+' when deciding if a value is an onum.

Eric Blake <ericb>
Group administrator
Sat 20 Apr 2013 05:01:21 PM UTC, comment #5: 

By my reading, '-perm +0100' is indeed unspecified by POSIX (POSIX only calls out symbolic mode, not numeric mode), so we get to choose how to handle it (the confusion with leading + is applicable to only things like '-perm +u+x').  I see two possible choices that make the most sense for how we should handle it: 1. make it behave the way it used to (no regression), or 2. make it return an error.  Having it silently change behavior from the old way is not nice.  I'm 60:40 towards changing it to be an error instead of fixing it to behave the way it used to.

Eric Blake <ericb>
Group administrator
Sat 20 Apr 2013 03:16:46 PM UTC, comment #4: 

James, the clause of the POSIX standard you mention is not relevant to this case because the mode was given in octal, not symbolic form.  The relevant clause is:

-perm [-]onum
    If the hyphen is omitted, the primary shall evaluate as true when the file permission bits exactly match the value of the octal number onum and only the bits corresponding to the octal mask 07777 shall be compared. (See the description of the octal mode in chmod().) Otherwise, if onum is prefixed by a hyphen, the primary shall evaluate as true if at least all of the bits specified in onum that are also set in the octal mask 07777 are set.

I take this to mean that "-perm +0100" is in fact unspecified.  Thus:

"-perm u+x", "-perm u=x" and "-perm 0100" match if and only if the file permissions are exactly --x------.

"-perm +u+x" and "-perm -0100" match if the file permissions are anything of the form ??x??????.

"-perm +0100" is unspecified and, as far as I understand, a syntax error.  Presumably, find should reject such an argument.

Ludovic Brenta <lbrenta>
Sat 20 Apr 2013 11:37:32 AM UTC, comment #3: 

The change was certainly unintended in the sense that when I updated the gnulib version, I did not expect this behaviour change.   But having re-read the POSIX standard, I think we have the surprising result that the new behaviour is correct and the previous behaviour isn't.  But it's possible I'm mistaken, so please read on (and check the POSIX standard).


I believe POSIX requires that "find . -perm +0100" matches only files which are mode 0100.   Not files which are executable by their owner but which also have other bits set.

Here's the description of the -perm predicate from POSIX (http://pubs.opengroup.org/onlinepubs/009695399/utilities/find.html):

---

-perm [-]mode
The mode argument is used to represent file mode bits. It shall be identical in format to the symbolic_mode operand described in chmod() , and shall be interpreted as follows. To start, a template shall be assumed with all file mode bits cleared. An op symbol of '+' shall set the appropriate mode bits in the template; '-' shall clear the appropriate bits; '=' shall set the appropriate mode bits, without regard to the contents of process' file mode creation mask. The op symbol of '-' cannot be the first character of mode; this avoids ambiguity with the optional leading hyphen. Since the initial mode is all bits off, there are not any symbolic modes that need to use '-' as the first character.
If the hyphen is omitted, the primary shall evaluate as true when the file permission bits exactly match the value of the resulting template.

Otherwise, if mode is prefixed by a hyphen, the primary shall evaluate as true if at least all the bits in the resulting template are set in the file permission bits.

---

The specified mode argument is "+0100" and it's not preceded by a leading hyphen.   Hence the mode bit 0100 (64 decimal) is turned on and the match is exact (since there was no leading "-").   So the predicate matches only files whose mode is exactly 0100.

I think this means that find-4.5.11 is correct, but previous versions of find were erroneously selecting the GNU extension behaviour (which is now selected by -perm /0100) when there is a correct but different POSIX behaviour.

Yes, this is surprising if you didn't know that "+" has been deprecated for a long time.   See this excerpt from the NEWS file:

  • Major changes in release 4.2.21, 2005-06-07
    • Functional Changes to find


The GNU extension "find ... -perm +MODE" has been withdrawn because it is incompatible with POSIX in obscure cases like "find ... -perm ++r". Use the new syntax "find ... -perm /MODE" instead.  Old usages will still continue to work, so long as they don't conflict with POSIX.

James Youngman <jay>
Group administrator
Sun 07 Apr 2013 10:10:38 AM UTC, comment #2: 

Hello,

the respective gnulib change which broke findutils is this one:
http://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=4730c3e3692b344effb72d46b3ff92db0bdb797a

4730c3e3692b344effb72d46b3ff92db0bdb797a
Modechange: add notations +40, 00440, etc.

Changelog entry:
modechange: add notations +40, 00440, etc.
lib/modechange.c (mode_compile): Support new notations
40, -40, =440, 00440.  See <http://debbugs.gnu.org/8391>.

cu andreas

Andreas Metzler <ametzler>
Sun 07 Apr 2013 08:58:31 AM UTC, comment #1: 

Hello,

this applies to both find and oldfind.

git bisect shows that it is triggered by commit bf5f83afdf9f26b6b44b942e0be122b7a66e3561 "find: use FTS_VERBATIM (and update gnulib)".

The culprit seems to be the gnulib update (from 372ef2a0e94ec6ee85b5fc4bab763154ec11420d to fd9f1acededd74f8cd095e657528aa8fad183536), reverting the other part of the commit does fix the bug.

cu andreas

Andreas Metzler <ametzler>
Wed 06 Mar 2013 08:50:37 AM UTC, original submission:  

It seems that in findutils 4.5.11 the find -perm +mode has broken (with a numerical mode). -perm /mode is still working as expected and as before.

See this example:

$ mkdir /tmp/foo
$ touch /tmp/foo/bar
$ chmod u+x /tmp/foo/bar

$ find --version | head -1
find (GNU findutils) 4.4.2
$ find /tmp/foo -perm +0100
/tmp/foo
/tmp/foo/bar
$ find /tmp/foo -perm /0100
/tmp/foo
/tmp/foo/bar

$ find --version | head -1
find (GNU findutils) 4.5.10
$ find /tmp/foo -perm +0100
/tmp/foo
/tmp/foo/bar
$ find /tmp/foo -perm /0100
/tmp/foo
/tmp/foo/bar


$ find --version | head -1
find (GNU findutils) 4.5.11
$ find /tmp/foo -perm +0100
$ find /tmp/foo -perm /0100
/tmp/foo
/tmp/foo/bar

Note that with version 4.5.11, -perm +0100 returns no files, even though there is an executable directory and file present.

Matthijs Kooijman <matthijs>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #27931:  havekind.txt added by eggert (3KiB - text/plain - More removal of support for -perm +MODE.)
file #27916:  patch.txt added by eggert (8KiB - text/plain - patch to make find -perm +600 an error)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by eggert (Updated the item)
  • -email is unavailable- added by ericb (Posted a comment)
  • -email is unavailable- added by lbrenta (Posted a comment)
  • -email is unavailable- added by jay (Posted a comment)
  • -email is unavailable- added by jay (Paul Eggert)
  • -email is unavailable- added by matthijs (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2013-04-24 jay StatusFixed None
    2013-04-22 eggert Attached File- Added havekind.txt, #27931
    2013-04-22 jay Attached File- Added 0001-Remove-support-for-obsolete-perm-MODE-syntax.patch, #27930
        StatusNone Fixed
    2013-04-21 eggert Attached File- Added patch.txt, #27916
    2013-04-20 jay Assigned toNone jay
        Carbon-Copy- Added -email is unavailable-
    2013-04-07 ametzler Carbon-CopyRemoved 20807 -

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code