[SSSD] [PATCH] File descriptor leak in nss responder.
Michal Židek
mzidek at redhat.com
Thu Mar 7 14:46:08 UTC 2013
On 03/07/2013 03:22 PM, Simo Sorce wrote:
> On Thu, 2013-03-07 at 15:02 +0100, Michal Židek wrote:
>> + /* Closing the file descriptor and ummaping the file
>> + * from memory is done in the mc_ctx_destructor. */
>> if (mc_ctx && mc_ctx->fd != -1) {
>> - close(mc_ctx->fd);
>> - ret = unlink(mc_ctx->file);
>> - if (ret == -1) {
>> - DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to rm mmap file %
>> s: %d(%s)\n",
>> - mc_ctx->file, ret,
>> strerror(ret)));
>> + dret = unlink(mc_ctx->file);
>> + if (dret == -1) {
>> + DEBUG(SSSDBG_CRIT_FAILURE,
>> + ("Failed to rm mmap file %s: %d(%s)\n",
>> mc_ctx->file,
>> + dret, strerror(dret)));
>> }
>
> sorry I just also realized this is probably incorrect, you should test
> mc_ctx->file not mc_ctx->fd
Not really. The flow is to allocate mc_ctx->file and after that call
open... so if fd is not -1 then the mc_ctx->file must be already
allocated (not null). Testing for mc_ctx->file would be incorrect,
because we might successfully create mc_ctx->file, but then fail during
open()... so checking for fd is correct here.
>
> I also see we have a minor issue in sss_mc_create_file(), we should
> probably remove close()/unlink() as well from the error condition in
> case sss_br_lock_file() fails, or at the very least set fd = -1 and file
> = NULL when close() and unlink() are called respectively.
Yes. But simply setting the fd = -1 should be enough (for the same
reason as described above).
I'll upload new patch soon.
>
> Simo.
>
More information about the sssd-devel
mailing list