Re: [Qemu-devel] [PATCH] qemu-char: fix tcp_get_fds

[Markus Armbruster]

"Michael S. Tsirkin" <address@hidden> writes:

> tcp_get_fds API discards fds if there's more than 1 of these.

s/tcp_get_fds/tcp_get_msgfds/ (subject as well)

What exactly doesn't work without this patch?

> It's tricky to fix this without API changes in the generic case.
>
> However, this API is only used by tests ATM, and tests know how
> many fds they expect.
>
> So let's not waste cycles trying to fix this properly:
> simply assume at most 16 fds (tests use at most 8 now).
> assert if some test tries to get more.
>
> Signed-off-by: Michael S. Tsirkin <address@hidden>
> ---
>  qemu-char.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index bd0709b..1c4004c 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -88,6 +88,7 @@
>  #define READ_BUF_LEN 4096
>  #define READ_RETRIES 10
>  #define CHR_MAX_FILENAME_SIZE 256
> +#define TCP_MAX_FDS 16
>  
>  /***********************************************************/
>  /* Socket address helpers */
> @@ -2668,6 +2669,8 @@ static int tcp_get_msgfds(CharDriverState *chr, int 
> *fds, int num)
>      TCPCharDriver *s = chr->opaque;
>      int to_copy = (s->read_msgfds_num < num) ? s->read_msgfds_num : num;
>  
> +    assert(num <= TCP_MAX_FDS);
> +
>      if (to_copy) {
>          int i;
>  

This where we copy received fds out of ->read_msgfds.  If someone asks
for more than TCP_MAX_FDS, the buffer in the next hunk is insufficient.

> @@ -2762,7 +2765,7 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char 
> *buf, size_t len)
>      struct iovec iov[1];
>      union {
>          struct cmsghdr cmsg;
> -        char control[CMSG_SPACE(sizeof(int))];
> +        char control[CMSG_SPACE(sizeof(int) * TCP_MAX_FDS)];
>      } msg_control;
>      int flags = 0;
>      ssize_t ret;

This is where we receive the fds into ->read_msgfds.  How many depends
on sizeof(msg_control).  One before your patch, TCP_MAX_FDS after.

Reviewed-by: Markus Armbruster <address@hidden>