Opened 16 months ago

Closed 15 months ago

Last modified 13 months ago

#8741 closed bug (fixed)

`System.Directory.getPermissions` fails on read-only filesystem

Reported by: hvr Owned by: AlainODea
Priority: high Milestone: 7.8.1
Component: libraries/unix Version: 7.8.1-rc1
Keywords: Cc:
Operating System: POSIX Architecture: Unknown/Multiple
Type of failure: Incorrect result at runtime Test Case:
Blocked By: Blocking:
Related Tickets: Differential Revisions:

Description

Alain O'Dea reports in an email that:

Prelude> System.Directory.getPermissions "/usr/bin/ld"
*** Exception: /usr/bin/ld: fileAccess: permission denied (Read-only file system)

That seems wrong.

An access(*, W_OK) syscall by design should return EROFS on a read-only file system by specification.

This breaks Cabal on SmartOS since /usr is read-only by design and Cabal calls getPermissions "/usr/bin/ld".

Attachments (1)

0001-BUGFIX-Handle-eROFS-as-permission-denied.patch (702 bytes) - added by AlainODea 16 months ago.
Patch including eROFS handling.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 16 months ago by AlainODea

  • Component changed from libraries/directory to libraries/unix
  • difficulty changed from Unknown to Easy (less than 1 hour)
  • Owner set to AlainODea

Changed 16 months ago by AlainODea

Patch including eROFS handling.

comment:2 Changed 16 months ago by AlainODea

  • Status changed from new to patch

comment:3 Changed 16 months ago by duncan

So the patch looks ok.

I think I would also add ETXTBSY as another alternative, i.e.

(err == eACCES || err == eROFS || err == eTXTBSY)

Since according to access(2)

       ETXTBSY
              Write  access was requested to an executable which is being exe‐
              cuted.

and open(2):

       ETXTBSY
              pathname refers to an executable image which is currently  being
              executed and write access was requested.

So the patch on its own is fine, but looking at System.Directory.getPermissions leaves me aghast:

  read_ok  <- Posix.fileAccess name True  False False
  write_ok <- Posix.fileAccess name False True  False
  exec_ok  <- Posix.fileAccess name False False True
  stat <- Posix.getFileStatus name

Four! Four? Four calls to stat? (access() is just a C lib function that calls stat().)

Surely we can do better than that? One call to stat should be enough.

Also, access doesn't do quite what we want. What we want to know is "if I tried to read/write/exec/traverse this file/dir, would it work?". But access answerss a subtly different question:

       The check is done using the calling process's real UID and GID,  rather
       than the effective IDs as is done when actually attempting an operation
       (e.g., open(2)) on the file.  This allows set-user-ID programs to  eas‐
       ily determine the invoking user's authority.

Well, that's all very good and useful, but it's not what we want here.

So yes, ideally we'd do one stat call and work it all out from there.

  stat <- Posix.getFileStatus name
  let mode = Posix.fileMode stat
      read_ok  = -- bit twiddling on mode using constants like Posix.ownerReadMode
      write_ok = -- etc
      exec_ok  = -- etc

comment:4 Changed 16 months ago by Herbert Valerio Riedel <hvr@…>

In ecc92abad017cf12d8eb83509d4d57ae14ad47f9/unix:

Handle EROFS/ETXTBSY as permission denied in `fileAccess` (re #8741)

This extends `System.Posix.Files.`access` to map EROFS & ETXTBSY to
mean permission denied just like EACCESS.

Based on a patch by Alain O'Dea and comments by Duncan Coutts

Authored-by: Alain O'Dea <[email protected]>
Signed-off-by: Herbert Valerio Riedel <[email protected]>

comment:5 Changed 16 months ago by thoughtpolice

  • Status changed from patch to merge

comment:6 Changed 15 months ago by thoughtpolice

  • Milestone set to 7.8.1
  • Version changed from 7.6.3 to 7.8.1-rc1

comment:7 Changed 15 months ago by thoughtpolice

  • Resolution set to fixed
  • Status changed from merge to closed

Merged in 7.8 for RC2.

comment:8 follow-up: Changed 15 months ago by AlainODea

Quick curiousity: why is this merged to 7.8 instead of a bugfix on 7.6.3? This makes cabal unusable on SmartOS when the packages being installed need to do native builds since it crashes on getPermissions to /usr/bin/ld. Haskell Platform 2013.2.0.0 has GHC 7.6.3. I think this means I can't use Haskell Platform on SmartOS without manually patching the unix library which seems hackish.

comment:9 in reply to: ↑ 8 ; follow-up: Changed 15 months ago by duncan

Replying to AlainODea:

Quick curiousity: why is this merged to 7.8 instead of a bugfix on 7.6.3?

I don't think there's any plan for another 7.6.x release (so no HP either), so there's not a lot of value in doing the extra work to backport it. If it's a particular problem for you and you can patch it then go ahead, it's perfectly reasonable to do so.

comment:10 in reply to: ↑ 9 Changed 15 months ago by AlainODea

Replying to duncan:

Replying to AlainODea:

Quick curiousity: why is this merged to 7.8 instead of a bugfix on 7.6.3?

I don't think there's any plan for another 7.6.x release (so no HP either), so there's not a lot of value in doing the extra work to backport it. If it's a particular problem for you and you can patch it then go ahead, it's perfectly reasonable to do so.

That makes sense. I'll patch manually. A new Haskell Platform is likely on deck with 7.8 nearing release now anyway so I'll focus my effort on making sure that works on SmartOS instead :)

comment:11 Changed 14 months ago by Austin Seipp <austin@…>

In 444750d2e5c6b843b937cbe876ce248e76a2a245/unix:

Handle EROFS/ETXTBSY as permission denied in `fileAccess` (re #8741)

This extends `System.Posix.Files.`access` to map EROFS & ETXTBSY to
mean permission denied just like EACCESS.

Based on a patch by Alain O'Dea and comments by Duncan Coutts

Authored-by: Alain O'Dea <[email protected]>
Signed-off-by: Herbert Valerio Riedel <[email protected]>

(cherry picked from commit ecc92abad017cf12d8eb83509d4d57ae14ad47f9)

comment:12 Changed 13 months ago by AlainODea

I have a backport of this fix in the pull request queue for the SmartOS PKGSRC package of GHC 7.6.3. That will resolve it for existing SmartOS users until I get 7.8 working.

https://github.com/joyent/pkgsrc-wip/pull/10

Note: See TracTickets for help on using tickets.