bugfindutils - Bugs: bug #54913, find: "-type"-test wrong...

 
 

bug #54913: find: "-type"-test wrong on bind-mounted char device

Submitted by:  Andreas Metzler <ametzler>
Submitted on:  Mon 29 Oct 2018 05:59:31 PM UTC  
 
Category:  find Severity:  3 - Normal
Item Group:  Wrong result Status:  None
Privacy:  Public Assigned to:  Bernhard Voelker <berny>
Originator Name:  Mathieu Parent Open/Closed:  Open
Release:  None Fixed Release:  None

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Fri 02 Nov 2018 01:49:04 PM UTC, comment #5:

Linux has similar behaviour about d_ino, returning the inode number from the parent filesystem rather than the mount point itself. This is a POSIX violation, but there's little interest in fixing it: https://groups.google.com/forum/#!topic/linux.kernel/4rzZRWKWT9c For similar reasons, I expect this behaviour won't change in the Linux kernel :(

So what to do? It's quite the pessimization to require stat() for -type. (Technically -type d can be answered without stat() because directories can only be mounted over directories.) I think trusting d_type for everything except mount points is a good idea.

Tavian Barnes <tavianator>
Thu 01 Nov 2018 07:01:07 PM UTC, comment #4:

What a pity: that code - to use the D_TYPE returned from gnulib's FTS - has
been added to avoid an extra stat() when getdents() already returns it:
https://git.sv.gnu.org/cgit/findutils.git/commit/?id=acb82fe

[adding Jim as the patch author]

The fix would be to undo this:

... but to the cost of performance again, because FTS usually returns
FTS_NSOK for all non-directory files (see "find -D search").
E.g. on NFS, that extra stat() would be quite slow ... and only needed
for that quite special and therefore seldom case of bind-mounts.

The only alternative I can currently think of is to check the mountlist
for the given ent->fts_path before not taking over the FTS information.

Any other idea to avoid the performance punishment?

Bernhard Voelker <berny>
Project AdministratorIn charge of this item.
Thu 01 Nov 2018 04:49:47 PM UTC, comment #3:

yes, getdents returns d_type=DT_REG, while the later newfstatat - needed for the -ls action - returns st_mode=S_IFCHR:

At least this is not an FTS regression, as 'oldfind' suffered from the same issue.

Bernhard Voelker <berny>
Project AdministratorIn charge of this item.
Wed 31 Oct 2018 08:57:18 PM UTC, comment #2:

So this issue basically happens because d_type comes from the parent filesystem right? Honestly this kernel behavior seems suboptimal to me. If it's too expensive to resolve d_type from the bind mount, it could at least return DT_UNKNOWN.

Tavian Barnes <tavianator>
Tue 30 Oct 2018 11:18:26 PM UTC, comment #1:

Nice catch - and a surprising use case of bind mounts!

And it's interesting that the output of the -ls action shows
the correct type again - of course, because that needs to do an
additional stat() on the file.

FWIW: this issue happens with all non-directory bind-mounts
on top of another type, e.g.
- socket -> regfile,
- regfile -> socket,
- character file -> socket
and so on.

I'll have to check if the change is done on the right place;
similar adjustment based on ent->ftsinfo are also in consider_visiting().

Furthermore, I'd like to get the coreutils-style test-framework
from gnulib into findutils, so that we could also have tests
like for this issue which require 'root' permissions.

Bernhard Voelker <berny>
Project AdministratorIn charge of this item.
Mon 29 Oct 2018 05:59:31 PM UTC, original submission:

Hello,

this was reported as https://bugs.debian.org/912180 by Mathieu Parent. He also provided the attached to fix the issue. I have verified the bug, and that the patch makes the issue unreproducible.
-----------------
Hi,

The following testcase:
i.e "-type f" wrongly matches a char device.

The attached patch fixes this.

Regards

Mathieu Parent

Andreas Metzler <ametzler>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #45323:  0001-Ensure-type-is-checked-with-FTS_NSOK-and-FTS_NS.patch added by ametzler (1KiB - text/x-diff - Patch provided in Debian report)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by berny
  • -email is unavailable- added by tavianator (Posted a comment)
  • -email is unavailable- added by berny (Posted a comment)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-11-01 berny Carbon-Copy- => Added meyering
    2018-10-31 ametzler Carbon-CopyRemoved 20807 => -
    2018-10-30 berny Assigned toNone => berny
    2018-10-29 ametzler Attached File- => Added 0001-Ensure-type-is-checked-with-FTS_NSOK-and-FTS_NS.patch, #45323

    Back to the top


    Powered by Savane 3.4