| 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
| ||
|
Message-ID: <97b602ea42b0467fa4e9f63a909f9080@chewa.net>
Date: Thu, 10 Nov 2011 11:36:49 +0100
From: Rémi Denis-Courmont <remi@...lab.net>
To: Hemant Vilas RAMDASI <hemant.ramdasi@...ricsson.com>
Cc: <remi.denis-courmont@...ia.com>, <netdev@...r.kernel.org>,
Dinesh Kumar Sharma <dinesh.sharma@...ricsson.com>
Subject: Re: [PATCH v2] Phonet: set the pipe handle using setsockopt
On Thu, 10 Nov 2011 15:20:22 +0530, Hemant Vilas RAMDASI
<hemant.ramdasi@...ricsson.com> wrote:
> From: Dinesh Kumar Sharma <dinesh.sharma@...ricsson.com>
>
> This provides flexibility to set the pipe handle
> using setsockopt and enable the same.
>
> Signed-off-by: Hemant Ramdasi <hemant.ramdasi@...ricsson.com>
> Signed-off-by: Dinesh Kumar Sharma <dinesh.sharma@...ricsson.com>
> ---
> include/linux/phonet.h | 2 +
> net/phonet/pep.c | 90
> ++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 90 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/phonet.h b/include/linux/phonet.h
> index 6fb1384..491caec 100644
> --- a/include/linux/phonet.h
> +++ b/include/linux/phonet.h
> @@ -37,6 +37,8 @@
> #define PNPIPE_ENCAP 1
> #define PNPIPE_IFINDEX 2
> #define PNPIPE_HANDLE 3
> +#define PNPIPE_ENABLE 4
> +#define PNPIPE_INITSTATE 5
>
> #define PNADDR_ANY 0
> #define PNADDR_BROADCAST 0xFC
> diff --git a/net/phonet/pep.c b/net/phonet/pep.c
> index f17fd84..f8057a1 100644
> --- a/net/phonet/pep.c
> +++ b/net/phonet/pep.c
> @@ -167,6 +167,12 @@ static int pipe_handler_send_created_ind(struct
sock
> *sk)
> data, 4, GFP_ATOMIC);
> }
>
> +static int pipe_handler_send_enabled_ind(struct sock *sk)
> +{
> + return pep_indicate(sk, PNS_PIPE_ENABLED_IND, 0 /* sub-blocks */,
> + NULL, 0, GFP_ATOMIC);
> +}
> +
> static int pep_accept_conn(struct sock *sk, struct sk_buff *skb)
> {
> static const u8 data[20] = {
> @@ -533,6 +539,17 @@ static int pep_connresp_rcv(struct sock *sk, struct
> sk_buff *skb)
> return pipe_handler_send_created_ind(sk);
> }
>
> +static int pep_enableresp_rcv(struct sock *sk, struct sk_buff *skb)
> +{
> + struct pnpipehdr *hdr = pnp_hdr(skb);
> +
> + if (hdr->error_code != PN_PIPE_NO_ERROR)
> + return -ECONNREFUSED;
> +
> + return pipe_handler_send_enabled_ind(sk);
> +}
> +
> +
> /* Queue an skb to an actively connected sock.
> * Socket lock must be held. */
> static int pipe_handler_do_rcv(struct sock *sk, struct sk_buff *skb)
> @@ -578,6 +595,28 @@ static int pipe_handler_do_rcv(struct sock *sk,
> struct sk_buff *skb)
> sk->sk_state = TCP_CLOSE_WAIT;
> break;
> }
> + if (pn->init_enable == PN_PIPE_DISABLE)
> + sk->sk_state = TCP_SYN_RECV;
> + else {
> + sk->sk_state = TCP_ESTABLISHED;
> +
> + if (!pn_flow_safe(pn->tx_fc)) {
> + atomic_set(&pn->tx_credits, 1);
> + sk->sk_write_space(sk);
> + }
> + pipe_grant_credits(sk, GFP_ATOMIC);
> +
> + }
I'd rather not duplicate this code as far as possible.
> + break;
> +
> + case PNS_PEP_ENABLE_RESP:
> + if (sk->sk_state != TCP_SYN_SENT)
> + break;
> +
> + if (pep_enableresp_rcv(sk, skb)) {
> + sk->sk_state = TCP_CLOSE_WAIT;
> + break;
> + }
>
> sk->sk_state = TCP_ESTABLISHED;
> if (!pn_flow_safe(pn->tx_fc)) {
> @@ -863,9 +902,26 @@ static int pep_sock_connect(struct sock *sk, struct
> sockaddr *addr, int len)
> int err;
> u8 data[4] = { 0 /* sub-blocks */, PAD, PAD, PAD };
>
> - pn->pipe_handle = 1; /* anything but INVALID_HANDLE */
> + if (pn->pipe_handle == PN_PIPE_INVALID_HANDLE)
> + pn->pipe_handle = 1; /* anything but INVALID_HANDLE */
> +
> err = pipe_handler_request(sk, PNS_PEP_CONNECT_REQ,
> - PN_PIPE_ENABLE, data, 4);
> + pn->init_enable, data, 4);
> + if (err)
> + return err;
> +
> + sk->sk_state = TCP_SYN_SENT;
> +
> + return 0;
> +}
> +
> +static int pep_sock_enable(struct sock *sk, struct sockaddr *addr, int
> len)
> +{
> + struct pep_sock *pn = pep_sk(sk);
> + int err;
> +
> + err = pipe_handler_request(sk, PNS_PEP_ENABLE_REQ, PAD,
> + NULL, 0);
> if (err) {
> pn->pipe_handle = PN_PIPE_INVALID_HANDLE;
> return err;
I doubt that a pipe in connected state with no handle is going to work.
> @@ -959,6 +1015,29 @@ static int pep_setsockopt(struct sock *sk, int
> level, int optname,
> }
> goto out_norel;
>
> + case PNPIPE_HANDLE:
> + if ((val >= 0) && (val < PN_PIPE_INVALID_HANDLE))
> + pn->pipe_handle = val;
> + else
> + err = -EINVAL;
> + break;
This should only be settable before connect(), I guess.
> +
> + case PNPIPE_ENABLE:
> + if (sk->sk_state == TCP_SYN_SENT)
> + err = -EBUSY;
This statement has no effects. You probably forgot something.
> + if (sk->sk_state == TCP_ESTABLISHED)
> + err = -EISCONN;
> + else
> + err = pep_sock_enable(sk, NULL, 0);
> + break;
This still does not follow the setter/getter level-trigger semantics of
(s|g)etsockopt().
> +
> + case PNPIPE_INITSTATE:
> + if ((val == PN_PIPE_DISABLE) || (val == PN_PIPE_ENABLE))
> + pn->init_enable = val;
> + else
> + err = -EINVAL;
IMHO, PNPIPE_INIT_ENABLE and boolean values would be simpler. I don't
really fancy exposing protocol-internal values to user space unless really
needed.
> + break;
> +
> default:
> err = -ENOPROTOOPT;
> }
> @@ -994,6 +1073,13 @@ static int pep_getsockopt(struct sock *sk, int
> level, int optname,
> return -EINVAL;
> break;
>
> + case PNPIPE_ENABLE:
> + if (sk->sk_state != TCP_ESTABLISHED)
> + return -EINVAL;
> + else
> + val = 1;
> + break;
This does not look correct.
> +
PNPIPE_INITSTATE is missing.
> default:
> return -ENOPROTOOPT;
> }
--
Rémi Denis-Courmont
http://www.remlab.net/
--
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
Powered by blists - more mailing lists