29 Mar, 2009

3 commits

  • check_unsafe_exec() also notes whether the fs_struct is being
    shared by more threads than will get killed by the exec, and if so
    sets LSM_UNSAFE_SHARE to make bprm_set_creds() careful about euid.
    But /proc//cwd and /proc//root lookups make transient
    use of get_fs_struct(), which also raises that sharing count.

    This might occasionally cause a setuid program not to change euid,
    in the same way as happened with files->count (check_unsafe_exec
    also looks at sighand->count, but /proc doesn't raise that one).

    We'd prefer exec not to unshare fs_struct: so fix this in procfs,
    replacing get_fs_struct() by get_fs_path(), which does path_get
    while still holding task_lock, instead of raising fs->count.

    Signed-off-by: Hugh Dickins
    Cc: stable@kernel.org
    ___

    fs/proc/base.c | 50 +++++++++++++++--------------------------------
    1 file changed, 16 insertions(+), 34 deletions(-)
    Signed-off-by: Linus Torvalds

    Hugh Dickins
     
  • Joe Malicki reports that setuid sometimes doesn't: very rarely,
    a setuid root program does not get root euid; and, by the way,
    they have a health check running lsof every few minutes.

    Right, check_unsafe_exec() notes whether the files_struct is being
    shared by more threads than will get killed by the exec, and if so
    sets LSM_UNSAFE_SHARE to make bprm_set_creds() careful about euid.
    But /proc//fd and /proc//fdinfo lookups make transient
    use of get_files_struct(), which also raises that sharing count.

    There's a rather simple fix for this: exec's check on files->count
    has been redundant ever since 2.6.1 made it unshare_files() (except
    while compat_do_execve() omitted to do so) - just remove that check.

    [Note to -stable: this patch will not apply before 2.6.29: earlier
    releases should just remove the files->count line from unsafe_exec().]

    Reported-by: Joe Malicki
    Narrowed-down-by: Michael Itz
    Tested-by: Joe Malicki
    Signed-off-by: Hugh Dickins
    Cc: stable@kernel.org
    Signed-off-by: Linus Torvalds

    Hugh Dickins
     
  • 2.6.26's commit fd8328be874f4190a811c58cd4778ec2c74d2c05
    "sanitize handling of shared descriptor tables in failing execve()"
    moved the unshare_files() from flush_old_exec() and several binfmts
    to the head of do_execve(); but forgot to make the same change to
    compat_do_execve(), leaving a CLONE_FILES files_struct shared across
    exec from a 32-bit process on a 64-bit kernel.

    It's arguable whether the files_struct really ought to be unshared
    across exec; but 2.6.1 made that so to stop the loading binary's fd
    leaking into other threads, and a 32-bit process on a 64-bit kernel
    ought to behave in the same way as 32 on 32 and 64 on 64.

    Signed-off-by: Hugh Dickins
    Cc: stable@kernel.org
    Signed-off-by: Linus Torvalds

    Hugh Dickins
     

28 Mar, 2009

37 commits