On Tue, Mar 10, 2020 at 11:26:23AM -0700, Elena Ufimtseva wrote:
> On Tue, Mar 10, 2020 at 04:09:41PM +0000, Stefan Hajnoczi wrote:
> > On Mon, Feb 24, 2020 at 03:54:58PM -0500, Jagannathan Raman wrote:
> > > +    msg->num_fds = 0;
> > > +    for (chdr = CMSG_FIRSTHDR(&hdr); chdr != NULL;
> > > +         chdr = CMSG_NXTHDR(&hdr, chdr)) {
> > > +        if ((chdr->cmsg_level == SOL_SOCKET) &&
> > > +            (chdr->cmsg_type == SCM_RIGHTS)) {
> > > +            fdsize = chdr->cmsg_len - CMSG_LEN(0);
> > > +            msg->num_fds = fdsize / sizeof(int);
> > > +            if (msg->num_fds > REMOTE_MAX_FDS) {
> > > +                /*
> > > +                 * TODO: Security issue detected. Sender never sends more
> > > +                 * than REMOTE_MAX_FDS. This condition should be signaled to
> > > +                 * the admin
> > > +                 */
> > 
> > This TODO doesn't seem actionable.  The error is already handled.
> > 
> > > +                qemu_log_mask(LOG_REMOTE_DEBUG,
> > > +                              "%s: Max FDs exceeded\n", __func__);
> > > +                return -ERANGE;
> > 
> > The mutex must be released.
> 
> Thank you! Will fix this and above.

I have posted a patch series that adds lock guards (automatic unlocking)
to prevent cases like this in the future:
https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg04628.html

You can use the QEMU_LOCK_GUARD() and/or WITH_QEMU_LOCK_GUARD() macros
to avoid the need for manual qemu_mutex_unlock() calls.

Stefan