linux-kernel - Re: [PATCH v4 12/18] xen/pvcalls: implement poll command

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 21 Jun 2017 16:55:56 -0400
From:   Boris Ostrovsky <boris.ostrovsky@...cle.com>
To:     Stefano Stabellini <sstabellini@...nel.org>
Cc:     xen-devel@...ts.xen.org, linux-kernel@...r.kernel.org,
        jgross@...e.com, Stefano Stabellini <stefano@...reto.com>
Subject: Re: [PATCH v4 12/18] xen/pvcalls: implement poll command


>>> +
>>> +	mappass->reqcopy = *req;
>>> +	icsk = inet_csk(mappass->sock->sk);
>>> +	queue = &icsk->icsk_accept_queue;
>>> +	spin_lock(&queue->rskq_lock);
>>> +	data = queue->rskq_accept_head != NULL;
>>> +	spin_unlock(&queue->rskq_lock);
>> What is the purpose of the queue lock here?
> It is only there to protect accesses to rskq_accept_head. Functions that
> change rskq_accept_head take this lock, see for example
> net/ipv4/inet_connection_sock.c:inet_csk_reqsk_queue_add. I'll add an
> in-code comment.

I am not sure I follow. You are not changing rskq_accept_head, you are
simply reading it under the lock. It may be set by others to NULL as
soon as you drop the lock, at which point 'data' test below will be
obsolete.

In inet_csk_reqsk_queue_add() it is read and then, based on read result,
is written with a value so a lock is indeed need there.

-boris

>
>
>>> +	if (data) {
>>> +		mappass->reqcopy.cmd = 0;
>>> +		ret = 0;
>>> +		goto out;
>>> +	}
>>> +	spin_unlock_irqrestore(&mappass->copy_lock, flags);
>>> +
>>> +	/* Tell the caller we don't need to send back a notification yet */
>>> +	return -1;