[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1298625301-13909-6-git-send-email-remi.denis-courmont@nokia.com>
Date: Fri, 25 Feb 2011 11:15:01 +0200
From: Rémi Denis-Courmont
<remi.denis-courmont@...ia.com>
To: netdev@...r.kernel.org
Subject: [PATCH 6/6] Phonet: fix flawed "SYN/ACK" logic
* Do not fail if the peer supports more or less than 3 algorithms.
* Ignore unknown congestion control algorithms instead of failing.
* Simplify congestion algorithm negotiation (largest is best).
* Do not use a static buffer.
* Fix off-by-two read overflow.
* Avoid extra memory copy (in addition to skb_copy_bits()).
The previous code really made no sense.
Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@...ia.com>
---
include/net/phonet/pep.h | 1 +
net/phonet/pep.c | 125 +++++++++++++++++----------------------------
2 files changed, 48 insertions(+), 78 deletions(-)
diff --git a/include/net/phonet/pep.h b/include/net/phonet/pep.h
index 37f23dc..38eed1b 100644
--- a/include/net/phonet/pep.h
+++ b/include/net/phonet/pep.h
@@ -154,6 +154,7 @@ enum {
PN_LEGACY_FLOW_CONTROL,
PN_ONE_CREDIT_FLOW_CONTROL,
PN_MULTI_CREDIT_FLOW_CONTROL,
+ PN_MAX_FLOW_CONTROL,
};
#define pn_flow_safe(fc) ((fc) >> 1)
diff --git a/net/phonet/pep.c b/net/phonet/pep.c
index 0ecab59..b8c31fc 100644
--- a/net/phonet/pep.c
+++ b/net/phonet/pep.c
@@ -108,70 +108,6 @@ static int pep_reply(struct sock *sk, struct sk_buff *oskb,
#define PAD 0x00
#ifdef CONFIG_PHONET_PIPECTRLR
-static u8 pipe_negotiate_fc(u8 *host_fc, u8 *remote_fc, int len)
-{
- int i, j;
- u8 base_fc, final_fc;
-
- for (i = 0; i < len; i++) {
- base_fc = host_fc[i];
- for (j = 0; j < len; j++) {
- if (remote_fc[j] == base_fc) {
- final_fc = base_fc;
- goto done;
- }
- }
- }
- return -EINVAL;
-
-done:
- return final_fc;
-
-}
-
-static int pipe_get_flow_info(struct sock *sk, struct sk_buff *skb,
- u8 *pref_rx_fc, u8 *req_tx_fc)
-{
- struct pnpipehdr *hdr;
- u8 n_sb;
-
- if (!pskb_may_pull(skb, sizeof(*hdr) + 4))
- return -EINVAL;
-
- hdr = pnp_hdr(skb);
- n_sb = hdr->data[4];
-
- __skb_pull(skb, sizeof(*hdr) + 4);
- while (n_sb > 0) {
- u8 type, buf[3], len = sizeof(buf);
- u8 *data = pep_get_sb(skb, &type, &len, buf);
-
- if (data == NULL)
- return -EINVAL;
-
- switch (type) {
- case PN_PIPE_SB_REQUIRED_FC_TX:
- if (len < 3 || (data[2] | data[3] | data[4]) > 3)
- break;
- req_tx_fc[0] = data[2];
- req_tx_fc[1] = data[3];
- req_tx_fc[2] = data[4];
- break;
-
- case PN_PIPE_SB_PREFERRED_FC_RX:
- if (len < 3 || (data[2] | data[3] | data[4]) > 3)
- break;
- pref_rx_fc[0] = data[2];
- pref_rx_fc[1] = data[3];
- pref_rx_fc[2] = data[4];
- break;
-
- }
- n_sb--;
- }
- return 0;
-}
-
static int pipe_handler_send_req(struct sock *sk, u8 msg_id, gfp_t priority)
{
int len;
@@ -661,28 +597,61 @@ static void pipe_destruct(struct sock *sk)
}
#ifdef CONFIG_PHONET_PIPECTRLR
+static u8 pipe_negotiate_fc(const u8 *fcs, unsigned n)
+{
+ unsigned i;
+ u8 final_fc = PN_NO_FLOW_CONTROL;
+
+ for (i = 0; i < n; i++) {
+ u8 fc = fcs[i];
+
+ if (fc > final_fc && fc < PN_MAX_FLOW_CONTROL)
+ final_fc = fc;
+ }
+ return final_fc;
+}
+
static int pep_connresp_rcv(struct sock *sk, struct sk_buff *skb)
{
struct pep_sock *pn = pep_sk(sk);
- u8 host_pref_rx_fc[3] = {3, 2, 1}, host_req_tx_fc[3] = {3, 2, 1};
- u8 remote_pref_rx_fc[3], remote_req_tx_fc[3];
- u8 negotiated_rx_fc, negotiated_tx_fc;
-
- pipe_get_flow_info(sk, skb, remote_pref_rx_fc,
- remote_req_tx_fc);
- negotiated_tx_fc = pipe_negotiate_fc(remote_req_tx_fc,
- host_pref_rx_fc,
- sizeof(host_pref_rx_fc));
- negotiated_rx_fc = pipe_negotiate_fc(host_req_tx_fc,
- remote_pref_rx_fc,
- sizeof(host_pref_rx_fc));
+ struct pnpipehdr *hdr;
+ u8 n_sb;
+
+ if (!pskb_pull(skb, sizeof(*hdr) + 4))
+ return -EINVAL;
+
+ hdr = pnp_hdr(skb);
+
+ /* Parse sub-blocks */
+ n_sb = hdr->data[4];
+ while (n_sb > 0) {
+ u8 type, buf[6], len = sizeof(buf);
+ const u8 *data = pep_get_sb(skb, &type, &len, buf);
+
+ if (data == NULL)
+ return -EINVAL;
+
+ switch (type) {
+ case PN_PIPE_SB_REQUIRED_FC_TX:
+ if (len < 2 || len < data[0])
+ break;
+ pn->tx_fc = pipe_negotiate_fc(data + 2, len - 2);
+ break;
+
+ case PN_PIPE_SB_PREFERRED_FC_RX:
+ if (len < 2 || len < data[0])
+ break;
+ pn->rx_fc = pipe_negotiate_fc(data + 2, len - 2);
+ break;
+
+ }
+ n_sb--;
+ }
sk->sk_state = TCP_SYN_RECV;
sk->sk_backlog_rcv = pipe_do_rcv;
sk->sk_destruct = pipe_destruct;
pn->rx_credits = 0;
- pn->rx_fc = negotiated_rx_fc;
- pn->tx_fc = negotiated_tx_fc;
sk->sk_state_change(sk);
return pipe_handler_send_created_ind(sk, PNS_PIPE_CREATED_IND);
--
1.7.1
--
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