Skip to content

pppd: fix memcpy overlap#579

Merged
paulusmack merged 1 commit intoppp-project:masterfrom
LGA1150:fix-memcpy-overlap
Mar 4, 2026
Merged

pppd: fix memcpy overlap#579
paulusmack merged 1 commit intoppp-project:masterfrom
LGA1150:fix-memcpy-overlap

Conversation

@LGA1150
Copy link
Copy Markdown
Contributor

@LGA1150 LGA1150 commented Feb 27, 2026

memcpy() with overlapping src and dest buffers is an undefined behavior in C. In the current code, a ConfRej response is generated by copying input data in-place, where the dest address is lower than the src. This happens to work in practice because memcpy() forward-copies data, matching the behavior of memmove() in this case.

However, if FORTIFY_SOURCE or Address Sanitizer is enabled, memcpy() will detect the overlap at run time and abort the program.

Replace the memcpy() with memmove() to ensure a well-defined behavior.

Closes: #576

memcpy() with overlapping src and dest buffers is an undefined behavior
in C. In the current code, a ConfRej response is generated by copying
input data in-place, where the dest address is lower than the src.
This happens to work in practice because memcpy() forward-copies data,
matching the behavior of memmove() in this case.

However, if FORTIFY_SOURCE or Address Sanitizer is enabled, memcpy()
will detect the overlap at run time and abort the program.

Replace the memcpy() with memmove() to ensure a well-defined behavior.

Reported-by: Filippo Carletti <filippo.carletti@gmail.com>
Closes: #576
Signed-off-by: Qingfang Deng <dqfext@gmail.com>
@paulusmack
Copy link
Copy Markdown
Collaborator

Seems reasonable, given that BSD bcopy() semantics allow overlapping source and destination.

@paulusmack paulusmack merged commit f8d9940 into ppp-project:master Mar 4, 2026
31 checks passed
@LGA1150 LGA1150 deleted the fix-memcpy-overlap branch March 5, 2026 01:14
@Neustradamus
Copy link
Copy Markdown
Member

@LGA1150: Thanks for your PR and @paulusmack for merging!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

pppd crashes with SIGILL (4)

3 participants