Skip to content

PAM/PASSKEY: avoid unnecessary memcpy#8630

Merged
alexey-tikhonov merged 1 commit intoSSSD:masterfrom
alexey-tikhonov:pamsrv-passkey-cleanup
Apr 22, 2026
Merged

PAM/PASSKEY: avoid unnecessary memcpy#8630
alexey-tikhonov merged 1 commit intoSSSD:masterfrom
alexey-tikhonov:pamsrv-passkey-cleanup

Conversation

@alexey-tikhonov
Copy link
Copy Markdown
Member

sss_authtok_set_passkey_reply() -> sss_authtok_set_string() handles non NULL-terminated buffer correctly.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the pam_passkey_child_read_data function in pamsrv_passkey.c to pass the data buffer directly to sss_authtok_set_passkey_reply. This change eliminates the need for manual memory allocation, copying, and null-termination of a temporary string. Additionally, the buffer length check was updated to handle non-positive values. I have no feedback to provide.

@alexey-tikhonov alexey-tikhonov added the coverity Trigger a coverity scan label Apr 22, 2026
Copy link
Copy Markdown
Contributor

@sumit-bose sumit-bose left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

thank you for the code-reduction, ACK.

bye,
Sumit

@alexey-tikhonov
Copy link
Copy Markdown
Member Author

Note: Covscan is clean.

@alexey-tikhonov alexey-tikhonov added Waiting for review and removed coverity Trigger a coverity scan labels Apr 22, 2026
Copy link
Copy Markdown
Contributor

@ikerexxe ikerexxe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

`sss_authtok_set_passkey_reply()` -> `sss_authtok_set_string()` handles
non NULL-terminated buffer correctly.

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Reviewed-by: Sumit Bose <sbose@redhat.com>
@sssd-bot
Copy link
Copy Markdown
Contributor

The pull request was accepted by @alexey-tikhonov with the following PR CI status:


🟢 CodeQL (success)
🟢 osh-diff-scan:fedora-rawhide-x86_64:upstream (success)
🟢 rpm-build:centos-stream-10-x86_64:upstream (success)
🟢 rpm-build:fedora-42-x86_64:upstream (success)
🟢 rpm-build:fedora-43-x86_64:upstream (success)
🟢 rpm-build:fedora-44-x86_64:upstream (success)
🟢 rpm-build:fedora-rawhide-x86_64:upstream (success)
🟢 Analyze (target) / cppcheck (success)
🟢 Build / freebsd (success)
🟢 Build / make-distcheck (success)
🟢 ci / intgcheck (centos-10) (success)
🟢 ci / intgcheck (fedora-42) (success)
🟢 ci / intgcheck (fedora-43) (success)
🟢 ci / intgcheck (fedora-44) (success)
🟢 ci / intgcheck (fedora-45) (success)
🟢 ci / prepare (success)
🟢 ci / system (centos-10) (success)
🟢 ci / system (fedora-42) (success)
🟢 ci / system (fedora-43) (success)
🟡 ci / system (fedora-44) (in_progress)
🟡 ci / system (fedora-45) (in_progress)
➖ Coverity scan / coverity (skipped)
🟢 Static code analysis / codeql (success)
🟢 Static code analysis / pre-commit (success)
🟢 Static code analysis / python-system-tests (success)


There are unsuccessful or unfinished checks. Make sure that the failures are not related to this pull request before merging.

@sssd-bot sssd-bot force-pushed the pamsrv-passkey-cleanup branch from 6dacebd to 6b6d131 Compare April 22, 2026 14:38
@alexey-tikhonov alexey-tikhonov merged commit 3b0b16e into SSSD:master Apr 22, 2026
10 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Accepted no-backport This should go to target branch only.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants