[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] check return value of fcntl() to detect invalid
From: |
Jason Wang |
Subject: |
Re: [Qemu-devel] [PATCH] check return value of fcntl() to detect invalid fd |
Date: |
Mon, 22 Dec 2014 13:54:20 +0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 |
On 12/22/2014 01:28 PM, Amos Kong wrote:
> On Mon, Dec 22, 2014 at 11:48:29AM +0800, Jason Wang wrote:
>> On 12/19/2014 09:25 PM, Amos Kong wrote:
>>> Passing some invalid fds in QEMU commandline, the fds don't exist.
>>> QEMU will get error "TUNGETIFF ioctl() failed: Bad file descriptor",
>>> and coredump in setting queues.
>>>
>>> This patch checked return value of first operate to fd, QEMU will
>>> report error and exit without coredump. It's effected for both netdev
>>> fds and vhost_net fds.
>>>
>>> Signed-off-by: Amos Kong <address@hidden>
>>> ---
>>> net/tap.c | 16 +++++++++++++---
>>> 1 file changed, 13 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/net/tap.c b/net/tap.c
>>> index bde6b58..039280a 100644
>>> --- a/net/tap.c
>>> +++ b/net/tap.c
>>> @@ -688,7 +688,7 @@ int net_init_tap(const NetClientOptions *opts, const
>>> char *name,
>>> NetClientState *peer)
>>> {
>>> const NetdevTapOptions *tap;
>>> - int fd, vnet_hdr = 0, i = 0, queues;
>>> + int fd, vnet_hdr = 0, i = 0, queues, ret;
>>> /* for the no-fd, no-helper case */
>>> const char *script = NULL; /* suppress wrong "uninit'd use" gcc
>>> warning */
>>> const char *downscript = NULL;
>>> @@ -722,7 +722,12 @@ int net_init_tap(const NetClientOptions *opts, const
>>> char *name,
>>> return -1;
>>> }
>>>
>>> - fcntl(fd, F_SETFL, O_NONBLOCK);
>>> + ret = fcntl(fd, F_SETFL, O_NONBLOCK);
>>> + if (ret < 0) {
>>> + error_report("Fail to set file status to nonblock, "
>>> + "%s", strerror(-ret));
>>> + return -1;
>>> + }
>> This may not work. There may be still some kinds of fd can pass this but
>> still fail at TUNGETIFF or other tun ioctls.
> Early catching the error is better. This only help to check if the fd
> exists.
If you just want to check the existence. Why don't you do it in
monitor_handle_fd_param() to let other case to benefit also? And
probably F_GETFL is better in this case.
But doing this does not solve the issue you mention in the commit log.
Even if fd exists, if it was not a tap fd, qemu may still abort.