netdev - Re: [PATCH 3/6] Phonet: Pipe End Point for Phonet Pipes protocol

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]

Message-Id: <200810021350.53010.remi.denis-courmont@nokia.com>
Date:	Thu, 2 Oct 2008 13:50:52 +0300
From:	"Rémi Denis-Courmont" 
	<remi.denis-courmont@...ia.com>
To:	"ext Arnaldo Carvalho de Melo" <acme@...hat.com>
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH 3/6] Phonet: Pipe End Point for Phonet Pipes protocol

On Wednesday 01 October 2008 16:18:56 ext Arnaldo Carvalho de Melo, you wrote:
> > +static struct sock *pep_find_pipe(const struct hlist_head *hlist,
> > +					const struct sockaddr_pn *dst,
> > +					u8 pipe_handle)
> > +{
> > +	struct hlist_node *node;
> > +	struct sock *sknode;
> > +	u16 dobj = pn_sockaddr_get_object(dst);
>
> What is the lock that protects this list traversal?

Either (accepted or unaccepted) sock lists should only be used with the sock 
lock of the listening sock. Is that insufficient?

> > +static int pep_wait_connreq(struct sock *sk, int noblock)
>
> This function looks familiar... inet_csk_accept,
> inet_csk_wait_for_connect...

I don't recall why I gave up on using request_sock and listen_sock there.

> perhaps we need a connection_sock father 
> for inet_connection_sock? :-)

But you cannot have double inheritance, right? inet_sock and 
connection_sock... I guess that's why listen_sock is _not_ a sock.

> > +{
> > +	struct task_struct *tsk = current;
> > +	struct pep_sock *pn = pep_sk(sk);
> > +	long timeo = sock_rcvtimeo(sk, noblock);
> > +
> > +	for (;;) {
> > +		DEFINE_WAIT(wait);
> > +
> > +		if (sk->sk_state != TCP_LISTEN)
> > +			return -EINVAL;
> > +		if (!hlist_empty(&pn->ackq))
> > +			break;
> > +		if (!timeo)
> > +			return -EWOULDBLOCK;
> > +		if (signal_pending(tsk))
> > +			return sock_intr_errno(timeo);
> > +
> > +		prepare_to_wait_exclusive(&sk->sk_socket->wait, &wait,
> > +						TASK_INTERRUPTIBLE);
> > +		release_sock(sk);
> > +		timeo = schedule_timeout(timeo);
> > +		lock_sock(sk);
> > +		finish_wait(&sk->sk_socket->wait, &wait);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static struct sock *pep_sock_accept(struct sock *sk, int flags, int
> > *errp) +{
> > +	struct pep_sock *pn = pep_sk(sk);
> > +	struct sock *newsk = NULL;
> > +	struct sk_buff *oskb;
> > +	int err;
> > +
> > +	lock_sock(sk);
> > +	err = pep_wait_connreq(sk, flags & O_NONBLOCK);
> > +	if (err)
> > +		goto out;
> > +
> > +	newsk = __sk_head(&pn->ackq);
> > +
> > +	oskb = skb_dequeue(&newsk->sk_receive_queue);
> > +	err = pep_accept_conn(newsk, oskb);
> > +	if (err) {
> > +		skb_queue_head(&newsk->sk_receive_queue, oskb);
> > +		newsk = NULL;
> > +		goto out;
> > +	}
> > +
> > +	sock_hold(sk);
> > +	pep_sk(newsk)->listener = sk;
> > +
> > +	sock_hold(newsk);
> > +	sk_del_node_init(newsk);
> > +	sk_acceptq_removed(sk);
> > +	sk_add_node(newsk, &pn->hlist);
> > +	__sock_put(newsk);
> > +
> > +out:
> > +	release_sock(sk);
> > +	*errp = err;
> > +	return newsk;
> > +}
> > +
> > +static int pep_ioctl(struct sock *sk, int cmd, unsigned long arg)
> > +{
> > +	int answ;
> > +
> > +	switch (cmd) {
> > +	case SIOCINQ:
> > +		if (sk->sk_state == TCP_LISTEN)
> > +			return -EINVAL;
> > +
> > +		lock_sock(sk);
> > +		if (!skb_queue_empty(&sk->sk_receive_queue))
> > +			answ = skb_peek(&sk->sk_receive_queue)->len;
> > +		else
> > +			answ = 0;
> > +		release_sock(sk);
> > +		return put_user(answ, (int __user *)arg);
>
> this is so common I wonder we if a helper wouldn't help 8) Look at
> dccp_ioctl before Ilpo does 8)

It is not so common with the next patch which checks for urgent inline... As 
far as I know, there is no common queue for out-of-band data.

-- 
Rémi Denis-Courmont
Maemo Software, Nokia Devices R&D
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html