Page MenuHomeFreeBSD

Call close(2) after errno is used.
ClosedPublic

Authored by kjopek_gmail.com on Dec 31 2020, 1:57 PM.
Tags
None
Referenced Files
F77769391: D27864.diff
Fri, Feb 23, 7:54 AM
Unknown Object (File)
Fri, Feb 9, 5:28 AM
Unknown Object (File)
Sat, Jan 27, 5:40 PM
Unknown Object (File)
Sat, Jan 27, 5:39 PM
Unknown Object (File)
Sat, Jan 27, 5:39 PM
Unknown Object (File)
Sat, Jan 27, 5:39 PM
Unknown Object (File)
Jan 23 2024, 3:35 AM
Unknown Object (File)
Dec 20 2023, 5:25 AM
Subscribers

Details

Summary

close(2) should be called after errno is examined as close may change errno variable.

Test Plan

No functional changes intended.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

In principle close() might obliterate errno, so rtld_strerror() call in dbg() inside the check becomes corrupted.

Oh, that means the current (in repo) code is wrong anyway. close(2) should be then called just before return.

kjopek_gmail.com retitled this revision from Reduce code duplication to Call close(2) after errno is used..
kjopek_gmail.com edited the summary of this revision. (Show Details)

Update patch accordingly to comments.

Well, then free() can obliterate errno as well.
I suggest to return to your initial patch, with some modifications.

Note that errno is not set by read(2) unless return value is -1, while code checks both for errors, EOFs, and short reads.
Save the value of errno somewhere if read returned -1, and then use the saved value in dbg().

Fix similar problems in different locations. Save original errno value from function it originally generated it.

Actually, we don't need to use errno_copy in first occurence of described problem.

Please use diff -U99999999 when generating patch, to add context to the review.

libexec/rtld-elf/libmap.c
106

We call such variable saved_errno in other places.

142

I suggest to issue different dbg() statements for case retval == -1 and retval >= 0. In the first case, use saved_errno, in second report something like 'short read' and print expected and returned lengths.

Provide different dbg messages for different types of errors.

Can you provide 'Submitted by:' line for the commit ?

This revision is now accepted and ready to land.Jan 1 2021, 10:07 PM

Submitted by: Konrad Sewiłło-Jopek

This revision was automatically updated to reflect the committed changes.