Fix resource leak on error in GetDevURandom #10837
Conversation
| do { | ||
| ssize_t n = read(f, ent32 + have, NUM_OS_RANDOM_BYTES - have); | ||
| if (n <= 0 || n + have > NUM_OS_RANDOM_BYTES) { | ||
| close(f); |
There was a problem hiding this comment.
Is it needed at all as RandFailure() aborts?
There was a problem hiding this comment.
As I understand it, most OS's will close descriptors on exit, but it is not guaranteed by the C language, so best practice is to close properly.
Also, depending on the behavior of RandFailure, which could change in the future, seems sloppy
There was a problem hiding this comment.
Heh, if we abort() there's gonna be a lot of things left open, but OK.
There was a problem hiding this comment.
I'm so used to try to bring everything down brick by brick, I can't remember ever resorting to abort, or even exit 😟
|
utACK a8ae0b2 +1 for explicit closing. Reduces the number of false positives when hunting file descriptor leaks. Thanks for a nice first-time contribution! |
| do { | ||
| ssize_t n = read(f, ent32 + have, NUM_OS_RANDOM_BYTES - have); | ||
| if (n <= 0 || n + have > NUM_OS_RANDOM_BYTES) { | ||
| close(f); |
There was a problem hiding this comment.
Heh, if we abort() there's gonna be a lot of things left open, but OK.
|
Follow-up PR: |
|
utACK a8ae0b2. Better to be explicit. |
|
utACK a8ae0b2. |
|
ACK a8ae0b2 |
a8ae0b2 Fix resource leak (Dag Robole) Pull request description: Fixes a potential file handle leak when size of entropy is invalid Tree-SHA512: 692d24daaf370bba1f842925b037275126f9494f54769650bcf5829c794a0fb8561a86f42347bdf088a484e4f107bce7fa14cd7bdbfb4ecfbeb51968953da3ae
a8ae0b2 Fix resource leak (Dag Robole) Pull request description: Fixes a potential file handle leak when size of entropy is invalid Tree-SHA512: 692d24daaf370bba1f842925b037275126f9494f54769650bcf5829c794a0fb8561a86f42347bdf088a484e4f107bce7fa14cd7bdbfb4ecfbeb51968953da3ae
Fixes a potential file handle leak when size of entropy is invalid