bugGNU Octave - Bugs: bug #57641, pkg install fails for noexec /tmp

 
 

bug #57641: pkg install fails for noexec /tmp

Submitter:  Daniel Kraft <domob>
Submitted:  Mon 20 Jan 2020 11:15:16 AM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Feature Request
Status:  Patch Submitted Assigned to:  None
Originator Name:  Open/Closed:  * Open
Release:  * 5.1.0 Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 30 Oct 2020 03:50:23 PM UTC, comment #14: 

I haven't tested the patch in comment #13 yet. But some preliminary comments:
IIUC, the used commands won't work cross-platform. Maybe `supports_execution` could return early with `true` if `isunix ()` is false.
You should check if the file handle `f` is valid before using it.
Maybe use an unwind_protect block around `system` and `shell` to unlink the file even if something went wrong.
Use a space after unary `!`.

Markus Mützel <mmuetzel>
Group administrator
Sun 22 Mar 2020 01:42:55 PM UTC, comment #13: 

Ah indeed, on a fresh build "setenv" works for me in this case.  I've reverted back the error message to the one suggested by Mike.

Thanks for pointing this out, Nicholas!

(file #48652)

Daniel Kraft <domob>
Sun 22 Mar 2020 11:59:33 AM UTC, comment #12: 

bug #55814 addressed the fact that some parts of the pkg build didn't respect setenv and relied directly on outside environment variables. But I thought that was fixed. Is that still the case here for you Daniel?

Nicholas Jankowski <nrjank>
Group Member
Sun 22 Mar 2020 09:24:29 AM UTC, comment #11: 

Thanks for the comments, Mike!  I've adapted my patch accordingly.  I've just adjusted the error message a bit more, namely to point out that the variable needs to be set outside of octave.  At least for me, setenv inside octave doesn't seem to work.

(file #48646)

Daniel Kraft <domob>
Sat 21 Mar 2020 08:22:04 PM UTC, comment #10: 

Thanks for writing the patch. This is not a full review, and I haven't tested, just a few "at a glance" comments.

There should be a space after the '!' unary operator.

The error message could be a little clearer, probably needs to be a little more verbose, something like "pkg: unable to execute temporary files in directory %s, try 'setenv TMPDIR ...' to use a different directory". We usually break long error messages like this


  error (["pkg: long error message " ...
          "continues here"]);


I think the call to 'chmod' could use 'system' instead of 'shell', it would be better to use 'system' when possible.

I'm also not sure how this will work on Windows, or whether it's even necessary, needs someone with Windows to test.

Also our commit messages are short and only describe "what" was changed, not "why" it was changed. See for example this commit message: https://hg.savannah.gnu.org/hgweb/octave/rev/3bf19af20af8

Mike Miller <mtmiller>
Group Member
Sat 21 Mar 2020 01:44:30 PM UTC, comment #9: 

I've created a first draft patch for "fixing" this.  Before executing a configure script, this change first writes a trivial shell script to the target directory and checks whether or not it can be executed.  If not, then an explicit error message is printed.

I do not know how this will behave on Windows, maybe we have to add a some "ispc" guard around it.  It does work for me on Debian.

Please let me know if there are any suggestions for changes to the patch, and I will be happy to do it (or feel free to do so yourself before pushing).

(file #48633)

Daniel Kraft <domob>
Thu 27 Feb 2020 07:56:08 PM UTC, comment #8: 

note that this came up a little while ago in bug #55646 on a Windows system.  Some security policies, especially on managed systems, restrict execution in the user temp folder on windows.  Had to set a default alternative tmpdir in my .octaverc (just created a C:\Octave\Temp folder) to be able to install many packages. it's been a trial and error troubleshooting case with subsequent users having the same problem. I don't know if there's an easy way to check for execute permission when it's not tied to filesystem properties. (e.g., my temp folder displays as Read & Execute permitted.)


Nicholas Jankowski <nrjank>
Group Member
Tue 21 Jan 2020 06:53:07 PM UTC, comment #7: 

If you want pkg to exit with an error message, it should only be in the case when a package build has a configure script. A simple makefile build should be assumed to work.

Mike Miller <mtmiller>
Group Member
Tue 21 Jan 2020 12:00:05 PM UTC, comment #6: 

@Daniel;
+1 for a pkg.m patch.
On Windows it's a bit easier as each user there has a scratch directory in the local profile (%HOMEDIR%/Appdata/Local/Temp); so Windows systems can be ignored explicitly when preparing a patch.

I'm not very knowledgeable with Linux system setup, but does Linux have alternative tmp-like locations for scratch files? If global /tmp is noexec (the reasons for which I understand very well) there might be one in the user's home dir.

@Mike:
Bug or not, I think pkg.m can & should do better than just blindly rely on an "exec" scratch dir these days; a point being that for unwary users the cause of the 'pkg install' failure might be hard to track down.
Octave uses /tmp for several other functions but TTBOMK those functions do not execute unpacked files.

Philip Nienhuis <philipnienhuis>
Group Member
Tue 21 Jan 2020 06:20:41 AM UTC, comment #5: 

To be honest, I'm not quite sure what can be done about this.  For one, just having the workaround documented here might help others who run into this and are looking for a fix.

But obviously the ideal situation is one where "pkg install" just works.  Given that this might be infeasible to do, maybe pkg could check if the temporary folder it uses has noexec, and if it does, display a proper error message and tell the user to apply TMPDIR.

I can try to work on a patch for this if you agree that's the best we can do.

Daniel Kraft <domob>
Mon 20 Jan 2020 06:58:56 PM UTC, comment #4: 

Yeah, setting TMPDIR is the best workaround for this.

If you consider this a bug, what kind of a solution do you think would fix this?

Mike Miller <mtmiller>
Group Member
Mon 20 Jan 2020 03:58:08 PM UTC, comment #3: 

Ah I see, I tried to set "TMPDIR" with "setenv" inside the Octave session.  But when I start Octave with that setting, I can also manipulate the output of "tempname()".

Mark as confirmed.

Kai Torben Ohlhus <siko1056>
Group Member
Mon 20 Jan 2020 03:49:44 PM UTC, comment #2: 

Actually, it seems to be possible to work around this issue with TMPDIR.  That's what I did in the end.

As I wrote in the OP, I still think this should be considered a "bug" - but it is probably not a very severe one and might be hard / infeasible to fix properly.

Daniel Kraft <domob>
Mon 20 Jan 2020 03:43:02 PM UTC, comment #1: 

Would it be more useful to make the `pkg` temporary directory configurable in this case?  `pkg` has `tempname()` hard coded which unfortunately is not manipulate able like `tempdir()` with the "$TMPDIR" environment variable.

https://hg.savannah.gnu.org/hgweb/octave/file/93f455582f60/scripts/pkg/pkg.m#l403

https://hg.savannah.gnu.org/hgweb/octave/file/93f455582f60/scripts/pkg/private/install.m#l67

https://octave.org/doc/v5.1.0/XREFtempname.html


Kai Torben Ohlhus <siko1056>
Group Member
Mon 20 Jan 2020 11:15:16 AM UTC, original submission:  

If /tmp is mounted noexec (which some people recommend as a security practice), then installing a package with "pkg install" fails.  The reason is that the installation procedure (in particular, configure_make.m) unpacks the package archive into /tmp and then tries to run "./configure".

This issue can easily be fixed by explicitly running the script with a shell (i.e. using "sh ./configure").  However, this still won't let me install the package successfully, because then configure itself complains that it can't run the generated C++ test program (error: cannot run C++ compiled programs).

So I'm not sure how feasible it is to fix this (but it seems like a minor bug / feature request anyway).

Daniel Kraft <domob>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #48652:  pkg-exec.patch added by domob (2KiB - text/x-patch - Patch with adjusted error message (pointing to setenv))
file #48646:  pkg-exec.patch added by domob (2KiB - text/x-patch - Patch adjusted for Mike's comments)
file #48633:  pkg-exec.patch added by domob (2KiB - text/x-patch - Proposed patch for pkg)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by nrjank (Posted a comment)
  • -email is unavailable- added by siko1056 (Posted a comment)
  • -email is unavailable- added by domob (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 group members can vote.

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-03-22 domob Attached File- Added pkg-exec.patch, #48652
    2020-03-22 domob Attached File- Added pkg-exec.patch, #48646
    2020-03-21 mtmiller StatusConfirmed Patch Submitted
    2020-03-21 domob Attached File- Added pkg-exec.patch, #48633
    2020-01-20 siko1056 StatusNeed Info Confirmed
    2020-01-20 siko1056 StatusNone Need Info

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code