• Eric W. Biederman's avatar
    ucounts: Move get_ucounts from cred_alloc_blank to key_change_session_keyring · 5ebcbe34
    Eric W. Biederman authored
    Setting cred->ucounts in cred_alloc_blank does not make sense.  The
    uid and user_ns are deliberately not set in cred_alloc_blank but
    instead the setting is delayed until key_change_session_keyring.
    
    So move dealing with ucounts into key_change_session_keyring as well.
    
    Unfortunately that movement of get_ucounts adds a new failure mode to
    key_change_session_keyring.  I do not see anything stopping the parent
    process from calling setuid and changing the relevant part of it's
    cred while keyctl_session_to_parent is running making it fundamentally
    necessary to call get_ucounts in key_change_session_keyring.  Which
    means that the new failure mode cannot be avoided.
    
    A failure of key_change_session_keyring results in a single threaded
    parent keeping it's existing credentials.  Which results in the parent
    process not being able to access the session keyring and whichever
    keys are in the new keyring.
    
    Further get_ucounts is only expected to fail if the number of bits in
    the refernece count for the structure is too few.
    
    Since the code has no other way to report the failure of get_ucounts
    and because such failures are not expected to be common add a WARN_ONCE
    to report this problem to userspace.
    
    Between the WARN_ONCE and the parent process not having access to
    the keys in the new session keyring I expect any failure of get_ucounts
    will be noticed and reported and we can find another way to handle this
    condition.  (Possibly by just making ucounts->count an atomic_long_t).
    
    Cc: stable@vger.kernel.org
    Fixes: 905ae01c ("Add a reference to ucounts for each cred")
    Link: https://lkml.kernel.org/r/7k0ias0uf.fsf_-_@disp2133Tested-by: default avatarYu Zhao <yuzhao@google.com>
    Reviewed-by: default avatarAlexey Gladkov <legion@kernel.org>
    Signed-off-by: default avatar"Eric W. Biederman" <ebiederm@xmission.com>
    5ebcbe34
cred.c 24.4 KB