Fix invalid file descriptor reuse while sending DNS query
Commit Message
When send_dg runs into a timeout it retries with RES_SNGLKUP and then
with RES_SNGLKUPREOP enabled. With the latter option the dns socket
will be reopend after the first query was sent, but the old file
descriptor is reused for sending the query, which can result in sending
it to an unrelated file in a multithreaded program.
Andreas.
[BZ #15946]
* resolv/res_send.c (send_dg): Reload file descriptor after
calling reopen.
---
resolv/res_send.c | 1 +
1 file changed, 1 insertion(+)
Comments
On 05/26/2014 12:35 PM, Andreas Schwab wrote:
> When send_dg runs into a timeout it retries with RES_SNGLKUP and then
> with RES_SNGLKUPREOP enabled. With the latter option the dns socket
> will be reopend after the first query was sent, but the old file
> descriptor is reused for sending the query, which can result in sending
> it to an unrelated file in a multithreaded program.
>
> Andreas.
>
> [BZ #15946]
> * resolv/res_send.c (send_dg): Reload file descriptor after
> calling reopen.
OK.
How did you test this? It requires simulating a DNS server with bad
behaviour to two outstanding requests, followed by a timeout. I'm happy
if this is just a fix done by inspection since I agree it looks correct.
I'm just curious. We need better DNS and network testing in glibc, any
ideas would be appreciated. I hope we can talk at Cauldron about this.
> ---
> resolv/res_send.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/resolv/res_send.c b/resolv/res_send.c
> index 3273d55..af42b8a 100644
> --- a/resolv/res_send.c
> +++ b/resolv/res_send.c
> @@ -1410,6 +1410,7 @@ send_dg(res_state statp,
> retval = reopen (statp, terrno, ns);
> if (retval <= 0)
> return retval;
> + pfd[0].fd = EXT(statp).nssocks[ns];
> }
> }
> goto wait;
>
Reviewed __res_iclose, reopen, and send_dg, looks correct to me.
Moving the setting of pfd[0] after `wait:' and reorganizing
a bit might have been an option, but it looked messier than your
fix.
Cheers,
Carlos.
"Carlos O'Donell" <carlos@redhat.com> writes:
> How did you test this?
See the bug entry.
Andreas.
@@ -1410,6 +1410,7 @@ send_dg(res_state statp,
retval = reopen (statp, terrno, ns);
if (retval <= 0)
return retval;
+ pfd[0].fd = EXT(statp).nssocks[ns];
}
}
goto wait;