Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

PAM: Fix unchecked return value from zfs_key_config_load() #13978

Merged
merged 1 commit into from
Oct 6, 2022
Merged

PAM: Fix unchecked return value from zfs_key_config_load() #13978

merged 1 commit into from
Oct 6, 2022

Conversation

ryao
Copy link
Contributor

@ryao ryao commented Oct 1, 2022

Motivation and Context

9a49c6b was intended to fix this issue, but I had missed the case in pam_sm_open_session(). Clang's static analyzer had not reported it and I forgot to look for other cases.

Interestingly, GCC 12.1.1's static analyzer had caught this as multiple double-free bugs, since another failure after the failure in zfs_key_config_load() will cause us to attempt to free the memory that zfs_key_config_load() was supposed to allocate, but had cleaned up upon failure.

Description

We check for a failure and return PAM_SESSION_ERR upon failure.

How Has This Been Tested?

The buildbot will test it.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

@ryao
PAM: Fix unchecked return value from zfs_key_config_load()
78808d8 
9a49c6b was intended to fix this issue,
but I had missed the case in pam_sm_open_session(). Clang's static
analyzer had not reported it and I forgot to look for other cases.

Interestingly, GCC gcc-12.1.1_p20220625's static analyzer had caught
this as multiple double-free bugs, since another failure after the
failure in zfs_key_config_load() will cause us to attempt to free the
memory that zfs_key_config_load() was supposed to allocate, but had
cleaned up upon failure.

Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
@behlendorf behlendorf merged commit 2ba240f into openzfs:master Oct 6, 2022
@behlendorf behlendorf added the Status: Accepted Ready to integrate (reviewed, tested) label Oct 6, 2022
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Oct 6, 2022
@ryao @andrewc12
PAM: Fix unchecked return value from zfs_key_config_load()
f19f80f 
9a49c6b was intended to fix this issue,
but I had missed the case in pam_sm_open_session(). Clang's static
analyzer had not reported it and I forgot to look for other cases.

Interestingly, GCC gcc-12.1.1_p20220625's static analyzer had caught
this as multiple double-free bugs, since another failure after the
failure in zfs_key_config_load() will cause us to attempt to free the
memory that zfs_key_config_load() was supposed to allocate, but had
cleaned up upon failure.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes openzfs#13978
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Oct 6, 2022
@ryao @andrewc12
PAM: Fix unchecked return value from zfs_key_config_load()
d49975e 
9a49c6b was intended to fix this issue,
but I had missed the case in pam_sm_open_session(). Clang's static
analyzer had not reported it and I forgot to look for other cases.

Interestingly, GCC gcc-12.1.1_p20220625's static analyzer had caught
this as multiple double-free bugs, since another failure after the
failure in zfs_key_config_load() will cause us to attempt to free the
memory that zfs_key_config_load() was supposed to allocate, but had
cleaned up upon failure.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes openzfs#13978
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Oct 6, 2022
@ryao @andrewc12
PAM: Fix unchecked return value from zfs_key_config_load()
0e3e9c8 
9a49c6b was intended to fix this issue,
but I had missed the case in pam_sm_open_session(). Clang's static
analyzer had not reported it and I forgot to look for other cases.

Interestingly, GCC gcc-12.1.1_p20220625's static analyzer had caught
this as multiple double-free bugs, since another failure after the
failure in zfs_key_config_load() will cause us to attempt to free the
memory that zfs_key_config_load() was supposed to allocate, but had
cleaned up upon failure.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes openzfs#13978
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Nov 23, 2022
@ryao @tonyhutter
PAM: Fix unchecked return value from zfs_key_config_load()
d923289 
9a49c6b was intended to fix this issue,
but I had missed the case in pam_sm_open_session(). Clang's static
analyzer had not reported it and I forgot to look for other cases.

Interestingly, GCC gcc-12.1.1_p20220625's static analyzer had caught
this as multiple double-free bugs, since another failure after the
failure in zfs_key_config_load() will cause us to attempt to free the
memory that zfs_key_config_load() was supposed to allocate, but had
cleaned up upon failure.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes openzfs#13978
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Nov 30, 2022
@ryao @tonyhutter
PAM: Fix unchecked return value from zfs_key_config_load()
d5412a8 
9a49c6b was intended to fix this issue,
but I had missed the case in pam_sm_open_session(). Clang's static
analyzer had not reported it and I forgot to look for other cases.

Interestingly, GCC gcc-12.1.1_p20220625's static analyzer had caught
this as multiple double-free bugs, since another failure after the
failure in zfs_key_config_load() will cause us to attempt to free the
memory that zfs_key_config_load() was supposed to allocate, but had
cleaned up upon failure.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes openzfs#13978
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Dec 1, 2022
@ryao @tonyhutter
PAM: Fix unchecked return value from zfs_key_config_load()
fa74250 
9a49c6b was intended to fix this issue,
but I had missed the case in pam_sm_open_session(). Clang's static
analyzer had not reported it and I forgot to look for other cases.

Interestingly, GCC gcc-12.1.1_p20220625's static analyzer had caught
this as multiple double-free bugs, since another failure after the
failure in zfs_key_config_load() will cause us to attempt to free the
memory that zfs_key_config_load() was supposed to allocate, but had
cleaned up upon failure.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes openzfs#13978
datacore-rm pushed a commit to DataCoreSoftware/openzfs that referenced this pull request Dec 22, 2022
@ryao @datacore-rm
PAM: Fix unchecked return value from zfs_key_config_load()
5e4a368 
9a49c6b was intended to fix this issue,
but I had missed the case in pam_sm_open_session(). Clang's static
analyzer had not reported it and I forgot to look for other cases.

Interestingly, GCC gcc-12.1.1_p20220625's static analyzer had caught
this as multiple double-free bugs, since another failure after the
failure in zfs_key_config_load() will cause us to attempt to free the
memory that zfs_key_config_load() was supposed to allocate, but had
cleaned up upon failure.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes openzfs#13978
datacore-rm pushed a commit to DataCoreSoftware/openzfs that referenced this pull request Dec 26, 2022
@ryao @datacore-rm
PAM: Fix unchecked return value from zfs_key_config_load()
720ad37 
9a49c6b was intended to fix this issue,
but I had missed the case in pam_sm_open_session(). Clang's static
analyzer had not reported it and I forgot to look for other cases.

Interestingly, GCC gcc-12.1.1_p20220625's static analyzer had caught
this as multiple double-free bugs, since another failure after the
failure in zfs_key_config_load() will cause us to attempt to free the
memory that zfs_key_config_load() was supposed to allocate, but had
cleaned up upon failure.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes openzfs#13978
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@behlendorf behlendorf behlendorf approved these changes

Assignees
No one assigned
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ryao @behlendorf