Opened 10 years ago

Last modified 2 years ago

#652 new defect

IZ2965: sge_peopen should be removed

Reported by: ernst Owned by:
Priority: normal Milestone:
Component: sge Version: 6.2u2
Severity: blocker Keywords: cleanup
Cc:

Description

[Imported from gridengine issuezilla http://gridengine.sunsource.net/issues/show_bug.cgi?id=2965]

        Issue #:      2965             Platform:     All      Reporter: ernst (ernst)
       Component:     gridengine          OS:        All
     Subcomponent:    cleanup          Version:      6.2u2       CC:    None defined
        Status:       NEW              Priority:     P3
      Resolution:                     Issue type:    DEFECT
                                   Target milestone: ---
      Assigned to:    ernst (ernst)
      QA Contact:     ernst
          URL:
       * Summary:     sge_peopen should be removed
   Status whiteboard:
      Attachments:

     Issue 2965 blocks:
   Votes for issue 2965:


   Opened: Wed Mar 25 08:32:00 -0700 2009 
------------------------


The function sge_peopen() might cause trouble in threaded environments when the process should not run with the same permission that the
calling process has. sge_peopen_r() has that possibility. Therefore sge_peopen() calls should be removed by sge_peopen_r() calls with the
next major release.

Attachments (1)

sge-peopen.patch (18.4 KB) - added by opoplawski 2 years ago.
Patch to drop sge_peopen()

Download all attachments as: .zip

Change History (2)

Changed 2 years ago by opoplawski

Patch to drop sge_peopen()

comment:1 Changed 2 years ago by opoplawski

  • Keywords removed
  • Severity set to blocker

I've been starting to poke some more into the sge code and have some questions and observations about the use of sge_peopen() as running external processes, especially when configured to run with an admin user.

This function is used by:

  • sge_execd to start the load sensor
  • various functions in sge/source/libs/gdi/sge_security.c to run security helper scripts: sge_set_cred()->get_token_cmd,get_cred, cache_sec_cred()->get_cred, delete_credentials()->delete_cred,store_sec_cred/2()->put_cred
  • jsv_start() -> JSV_command
  • sge_afs_extend_token(command) -> command
  • sge_get_pids(pscommand) -> pscommand
  • sge_checkprog(pscommon) -> pscommand

Notably it is not used to launch jobs.

The current behavior of sge_peopen_r() is to switch back to the root (or the user that started the sge_execd/qmaster command) before spawning the command. Notably this results in load sensors being run as root, which strikes me as a very bad idea.

This patch also changes peopen's behavior to only switch root if it was requested to change the user, which currently none of the callers do. This now has the load sensor running as sgeadmin.

I also changed sge_qmaster on my install to startup as the sgeadmin user by adding:

User=sgeadmin

to the sge_qmaster.service unit file. So far I haven't noticed any issues.

Note: See TracTickets for help on using tickets.