Page MenuHomeFreeBSD

Call close(2) after errno is used.
ClosedPublic

Authored by kjopek_gmail.com on Dec 31 2020, 1:57 PM.

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
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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.