* Re: pull request for net: batman-adv 2013-08-10
From: David Miller @ 2013-08-10 22:39 UTC (permalink / raw)
To: ordex; +Cc: netdev, b.a.t.m.a.n
In-Reply-To: <1376168881-4079-1-git-send-email-ordex@autistici.org>
From: Antonio Quartulli <ordex@autistici.org>
Date: Sat, 10 Aug 2013 23:08:00 +0200
> here you have one fix intended for net/linux-3.11.
>
> To avoid kernel paging errors, this patch takes care of re-assigning the value
> of skb->data to the related pointers after that the skb has been re-allocated.
>
> Please pull or let me know of any problem!
Pulled, thanks.
^ permalink raw reply
* [PATCH net-next] pptp: fix byte order warnings
From: Stephen Hemminger @ 2013-08-10 22:22 UTC (permalink / raw)
To: David Miller, Dmitry Kozlov; +Cc: netdev
Pptp driver has lots of byte order warnings from sparse.
This was because the on-the-wire header is in network byte order (obviously)
but the definition did not reflect that.
Also, the address structure to user space actually put the call id
in host order. Rather than break ABI compatibility, just acknowledge
the existing design.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
--- a/drivers/net/ppp/pptp.c 2013-08-10 10:36:10.365515260 -0700
+++ b/drivers/net/ppp/pptp.c 2013-08-10 15:17:38.514965016 -0700
@@ -83,11 +83,11 @@ static const struct proto_ops pptp_ops;
struct pptp_gre_header {
u8 flags;
u8 ver;
- u16 protocol;
- u16 payload_len;
- u16 call_id;
- u32 seq;
- u32 ack;
+ __be16 protocol;
+ __be16 payload_len;
+ __be16 call_id;
+ __be32 seq;
+ __be32 ack;
} __packed;
static struct pppox_sock *lookup_chan(u16 call_id, __be32 s_addr)
--- a/include/uapi/linux/if_pppox.h 2013-08-10 10:36:11.477500663 -0700
+++ b/include/uapi/linux/if_pppox.h 2013-08-10 15:10:40.093460682 -0700
@@ -46,7 +46,7 @@ struct pppoe_addr {
* PPTP addressing definition
*/
struct pptp_addr {
- __be16 call_id;
+ __u16 call_id;
struct in_addr sin_addr;
};
^ permalink raw reply
* [PATCH net] skge: dma_sync the whole receive buffer
From: Stephen Hemminger @ 2013-08-10 22:02 UTC (permalink / raw)
To: David Miller; +Cc: pomidorabelisima, netdev
In-Reply-To: <20130810.132935.1257046025460198490.davem@davemloft.net>
The DMA sync should sync the whole receive buffer, not just
part of it. Fixes log messages dma_sync_check.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
--- a/drivers/net/ethernet/marvell/skge.c 2013-08-10 14:54:13.184737163 -0700
+++ b/drivers/net/ethernet/marvell/skge.c 2013-08-10 14:54:54.908099676 -0700
@@ -3077,11 +3077,13 @@ static struct sk_buff *skge_rx_get(struc
pci_dma_sync_single_for_cpu(skge->hw->pdev,
dma_unmap_addr(e, mapaddr),
- len, PCI_DMA_FROMDEVICE);
+ dma_unmap_len(e, maplen),
+ PCI_DMA_FROMDEVICE);
skb_copy_from_linear_data(e->skb, skb->data, len);
pci_dma_sync_single_for_device(skge->hw->pdev,
dma_unmap_addr(e, mapaddr),
- len, PCI_DMA_FROMDEVICE);
+ dma_unmap_len(e, maplen),
+ PCI_DMA_FROMDEVICE);
skge_rx_reuse(e, skge->rx_buf_size);
} else {
struct sk_buff *nskb;
^ permalink raw reply
* pull request for net: batman-adv 2013-08-10
From: Antonio Quartulli @ 2013-08-10 21:08 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n
Hello David,
here you have one fix intended for net/linux-3.11.
To avoid kernel paging errors, this patch takes care of re-assigning the value
of skb->data to the related pointers after that the skb has been re-allocated.
Please pull or let me know of any problem!
Thank you,
Antonio
The following changes since commit 645359930231d5e78fd3296a38b98c1a658a7ade:
rtnetlink: Fix inverted check in ndo_dflt_fdb_del() (2013-08-10 01:24:12 -0700)
are available in the git repository at:
git://git.open-mesh.org/linux-merge.git tags/batman-adv-fix-for-davem
for you to fetch changes up to 9d2c9488cedb666bc8206fbdcdc1575e0fbc5929:
batman-adv: fix potential kernel paging errors for unicast transmissions (2013-08-10 22:55:42 +0200)
----------------------------------------------------------------
Included change:
- reassign pointers to data after skb reallocation to avoid kernel paging errors
----------------------------------------------------------------
Linus Lüssing (1):
batman-adv: fix potential kernel paging errors for unicast transmissions
net/batman-adv/bridge_loop_avoidance.c | 2 ++
net/batman-adv/gateway_client.c | 13 ++++++++++++-
net/batman-adv/gateway_client.h | 3 +--
net/batman-adv/soft-interface.c | 9 ++++++++-
net/batman-adv/unicast.c | 13 ++++++++++---
5 files changed, 33 insertions(+), 7 deletions(-)
^ permalink raw reply
* [PATCH] batman-adv: fix potential kernel paging errors for unicast transmissions
From: Antonio Quartulli @ 2013-08-10 21:08 UTC (permalink / raw)
To: davem
Cc: netdev, b.a.t.m.a.n, Linus Lüssing, Marek Lindner,
Antonio Quartulli
In-Reply-To: <1376168881-4079-1-git-send-email-ordex@autistici.org>
From: Linus Lüssing <linus.luessing@web.de>
There are several functions which might reallocate skb data. Currently
some places keep reusing their old ethhdr pointer regardless of whether
they became invalid after such a reallocation or not. This potentially
leads to kernel paging errors.
This patch fixes these by refetching the ethdr pointer after the
potential reallocations.
Signed-off-by: Linus Lüssing <linus.luessing@web.de>
Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
net/batman-adv/bridge_loop_avoidance.c | 2 ++
net/batman-adv/gateway_client.c | 13 ++++++++++++-
net/batman-adv/gateway_client.h | 3 +--
net/batman-adv/soft-interface.c | 9 ++++++++-
net/batman-adv/unicast.c | 13 ++++++++++---
5 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
index e14531f..264de88 100644
--- a/net/batman-adv/bridge_loop_avoidance.c
+++ b/net/batman-adv/bridge_loop_avoidance.c
@@ -1529,6 +1529,8 @@ out:
* in these cases, the skb is further handled by this function and
* returns 1, otherwise it returns 0 and the caller shall further
* process the skb.
+ *
+ * This call might reallocate skb data.
*/
int batadv_bla_tx(struct batadv_priv *bat_priv, struct sk_buff *skb,
unsigned short vid)
diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c
index f105219..7614af3 100644
--- a/net/batman-adv/gateway_client.c
+++ b/net/batman-adv/gateway_client.c
@@ -508,6 +508,7 @@ out:
return 0;
}
+/* this call might reallocate skb data */
static bool batadv_is_type_dhcprequest(struct sk_buff *skb, int header_len)
{
int ret = false;
@@ -568,6 +569,7 @@ out:
return ret;
}
+/* this call might reallocate skb data */
bool batadv_gw_is_dhcp_target(struct sk_buff *skb, unsigned int *header_len)
{
struct ethhdr *ethhdr;
@@ -619,6 +621,12 @@ bool batadv_gw_is_dhcp_target(struct sk_buff *skb, unsigned int *header_len)
if (!pskb_may_pull(skb, *header_len + sizeof(*udphdr)))
return false;
+
+ /* skb->data might have been reallocated by pskb_may_pull() */
+ ethhdr = (struct ethhdr *)skb->data;
+ if (ntohs(ethhdr->h_proto) == ETH_P_8021Q)
+ ethhdr = (struct ethhdr *)(skb->data + VLAN_HLEN);
+
udphdr = (struct udphdr *)(skb->data + *header_len);
*header_len += sizeof(*udphdr);
@@ -634,12 +642,14 @@ bool batadv_gw_is_dhcp_target(struct sk_buff *skb, unsigned int *header_len)
return true;
}
+/* this call might reallocate skb data */
bool batadv_gw_out_of_range(struct batadv_priv *bat_priv,
- struct sk_buff *skb, struct ethhdr *ethhdr)
+ struct sk_buff *skb)
{
struct batadv_neigh_node *neigh_curr = NULL, *neigh_old = NULL;
struct batadv_orig_node *orig_dst_node = NULL;
struct batadv_gw_node *curr_gw = NULL;
+ struct ethhdr *ethhdr;
bool ret, out_of_range = false;
unsigned int header_len = 0;
uint8_t curr_tq_avg;
@@ -648,6 +658,7 @@ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv,
if (!ret)
goto out;
+ ethhdr = (struct ethhdr *)skb->data;
orig_dst_node = batadv_transtable_search(bat_priv, ethhdr->h_source,
ethhdr->h_dest);
if (!orig_dst_node)
diff --git a/net/batman-adv/gateway_client.h b/net/batman-adv/gateway_client.h
index 039902d..1037d75 100644
--- a/net/batman-adv/gateway_client.h
+++ b/net/batman-adv/gateway_client.h
@@ -34,7 +34,6 @@ void batadv_gw_node_delete(struct batadv_priv *bat_priv,
void batadv_gw_node_purge(struct batadv_priv *bat_priv);
int batadv_gw_client_seq_print_text(struct seq_file *seq, void *offset);
bool batadv_gw_is_dhcp_target(struct sk_buff *skb, unsigned int *header_len);
-bool batadv_gw_out_of_range(struct batadv_priv *bat_priv,
- struct sk_buff *skb, struct ethhdr *ethhdr);
+bool batadv_gw_out_of_range(struct batadv_priv *bat_priv, struct sk_buff *skb);
#endif /* _NET_BATMAN_ADV_GATEWAY_CLIENT_H_ */
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index 700d0b4..0f04e1c 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -180,6 +180,9 @@ static int batadv_interface_tx(struct sk_buff *skb,
if (batadv_bla_tx(bat_priv, skb, vid))
goto dropped;
+ /* skb->data might have been reallocated by batadv_bla_tx() */
+ ethhdr = (struct ethhdr *)skb->data;
+
/* Register the client MAC in the transtable */
if (!is_multicast_ether_addr(ethhdr->h_source))
batadv_tt_local_add(soft_iface, ethhdr->h_source, skb->skb_iif);
@@ -220,6 +223,10 @@ static int batadv_interface_tx(struct sk_buff *skb,
default:
break;
}
+
+ /* reminder: ethhdr might have become unusable from here on
+ * (batadv_gw_is_dhcp_target() might have reallocated skb data)
+ */
}
/* ethernet packet should be broadcasted */
@@ -266,7 +273,7 @@ static int batadv_interface_tx(struct sk_buff *skb,
/* unicast packet */
} else {
if (atomic_read(&bat_priv->gw_mode) != BATADV_GW_MODE_OFF) {
- ret = batadv_gw_out_of_range(bat_priv, skb, ethhdr);
+ ret = batadv_gw_out_of_range(bat_priv, skb);
if (ret)
goto dropped;
}
diff --git a/net/batman-adv/unicast.c b/net/batman-adv/unicast.c
index dc8b5d4..688a041 100644
--- a/net/batman-adv/unicast.c
+++ b/net/batman-adv/unicast.c
@@ -326,7 +326,9 @@ static bool batadv_unicast_push_and_fill_skb(struct sk_buff *skb, int hdr_size,
* @skb: the skb containing the payload to encapsulate
* @orig_node: the destination node
*
- * Returns false if the payload could not be encapsulated or true otherwise
+ * Returns false if the payload could not be encapsulated or true otherwise.
+ *
+ * This call might reallocate skb data.
*/
static bool batadv_unicast_prepare_skb(struct sk_buff *skb,
struct batadv_orig_node *orig_node)
@@ -343,7 +345,9 @@ static bool batadv_unicast_prepare_skb(struct sk_buff *skb,
* @orig_node: the destination node
* @packet_subtype: the batman 4addr packet subtype to use
*
- * Returns false if the payload could not be encapsulated or true otherwise
+ * Returns false if the payload could not be encapsulated or true otherwise.
+ *
+ * This call might reallocate skb data.
*/
bool batadv_unicast_4addr_prepare_skb(struct batadv_priv *bat_priv,
struct sk_buff *skb,
@@ -401,7 +405,7 @@ int batadv_unicast_generic_send_skb(struct batadv_priv *bat_priv,
struct batadv_neigh_node *neigh_node;
int data_len = skb->len;
int ret = NET_RX_DROP;
- unsigned int dev_mtu;
+ unsigned int dev_mtu, header_len;
/* get routing information */
if (is_multicast_ether_addr(ethhdr->h_dest)) {
@@ -429,10 +433,12 @@ find_router:
switch (packet_type) {
case BATADV_UNICAST:
batadv_unicast_prepare_skb(skb, orig_node);
+ header_len = sizeof(struct batadv_unicast_packet);
break;
case BATADV_UNICAST_4ADDR:
batadv_unicast_4addr_prepare_skb(bat_priv, skb, orig_node,
packet_subtype);
+ header_len = sizeof(struct batadv_unicast_4addr_packet);
break;
default:
/* this function supports UNICAST and UNICAST_4ADDR only. It
@@ -441,6 +447,7 @@ find_router:
goto out;
}
+ ethhdr = (struct ethhdr *)(skb->data + header_len);
unicast_packet = (struct batadv_unicast_packet *)skb->data;
/* inform the destination node that we are still missing a correct route
--
1.8.1.5
^ permalink raw reply related
* Re: [PATCH 0/4] netfilter fixes for 3.11-rc4
From: David Miller @ 2013-08-10 20:44 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <1376153933-5747-1-git-send-email-pablo@netfilter.org>
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Sat, 10 Aug 2013 18:58:49 +0200
> The following patchset contains four netfilter fixes, they are:
>
> * Fix possible invalid access and mangling of the TCPMSS option in
> xt_TCPMSS. This was spotted by Julian Anastasov.
>
> * Fix possible off by one access and mangling of the TCP packet in
> xt_TCPOPTSTRIP, also spotted by Julian Anastasov.
>
> * Fix possible information leak due to missing initialization of one
> padding field of several structures that are included in nfqueue and
> nflog netlink messages, from Dan Carpenter.
>
> * Fix TCP window tracking with Fast Open, from Yuchung Cheng.
>
> You can pull these changes from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git master
Pulled, thanks Pablo!
^ permalink raw reply
* Re: [PATCH net] skge: add dma_mapping check
From: David Miller @ 2013-08-10 20:29 UTC (permalink / raw)
To: stephen; +Cc: pomidorabelisima, netdev
In-Reply-To: <20130810104100.0ae20aa6@nehalam.linuxnetplumber.net>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Sat, 10 Aug 2013 10:41:00 -0700
> The DMA debug check insists that the call to sync_single has
> to be the same length as the mapping. The driver is only bothering
> to sync the bytes that matter (got received) rather than the whole
> map.
>
> In reality that is not required, but I will make a patch anyway.
PCI and other system controllers will prefetch, I think it can matter.
^ permalink raw reply
* Re: [PATCH] drivers:net:hamradio: Removing Depricated IRQF_DISABLED
From: Kumar Gaurav @ 2013-08-10 18:53 UTC (permalink / raw)
To: wharms
Cc: Julia Lawall, t.sailer, jreuter, jpr, linux-hams, netdev,
linux-kernel, kernel-janitors
In-Reply-To: <52068B3C.4030707@bfs.de>
On Sunday 11 August 2013 12:19 AM, walter harms wrote:
> Hello Kumar,
> i will try to resolve the riddle ...
>
> doing something like fn(a,b,c,0) is a bad habit.
> 0 is called a "magic number" because noone reading the code has an idea
> why it is there. Using the name IRQF_TRIGGER_NONE good.
>
> Having a named bit like IRQ_TRIGGER|IRQ_NMI is most times also a good idea
> because it correspond (most times) to the naming of the bits in the data sheet.
>
> The problems starts when this hits reality since nothing is clearly black or white.
> In this case when you remove IRQF_DISABLED disables "bit" something should be left.
>
> a|b|IRQF_DISABLED -> a|b (that is the easy part)
> but
> IRQF_DISABLED -> IRQF_TRIGGER_NONE
>
> Otherwise you would be left with "0". That is a magic number (see above).
> So this is to be replaced by a placeholder called IRQF_TRIGGER_NONE.
>
> hope that really helps and that i have understood the discussion.
>
> re,
> wh
Thanks for the clarification. I'll be following this.
Regards
Kumar Gaurav
>
> Am 10.08.2013 20:15, schrieb Kumar Gaurav:
>> On Saturday 10 August 2013 10:54 PM, Julia Lawall wrote:
>>> On Sat, 10 Aug 2013, Kumar Gaurav wrote:
>>>
>>>> On Saturday 10 August 2013 10:17 PM, Julia Lawall wrote:
>>>>> If it is in a | with something else, I think you can just remove it.
>>>>>
>>>>> julia
>>>>>
>>>>> On Sat, 10 Aug 2013, Kumar Gaurav wrote:
>>>>>
>>>>>> Removed IRQF_DISABLED as it's deprecated and should be removed
>>>>>>
>>>>>> Signed-off-by: Kumar Gaurav <kumargauravgupta3@gmail.com>
>>>>>> ---
>>>>>> drivers/net/hamradio/baycom_ser_fdx.c | 2 +-
>>>>>> drivers/net/hamradio/baycom_ser_hdx.c | 2 +-
>>>>>> drivers/net/hamradio/scc.c | 2 +-
>>>>>> drivers/net/hamradio/yam.c | 2 +-
>>>>>> 4 files changed, 4 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/net/hamradio/baycom_ser_fdx.c
>>>>>> b/drivers/net/hamradio/baycom_ser_fdx.c
>>>>>> index a974727..c114009 100644
>>>>>> --- a/drivers/net/hamradio/baycom_ser_fdx.c
>>>>>> +++ b/drivers/net/hamradio/baycom_ser_fdx.c
>>>>>> @@ -445,7 +445,7 @@ static int ser12_open(struct net_device *dev)
>>>>>> outb(0, FCR(dev->base_addr)); /* disable FIFOs */
>>>>>> outb(0x0d, MCR(dev->base_addr));
>>>>>> outb(0, IER(dev->base_addr));
>>>>>> - if (request_irq(dev->irq, ser12_interrupt, IRQF_DISABLED |
>>>>>> IRQF_SHARED,
>>>>>> + if (request_irq(dev->irq, ser12_interrupt, IRQF_TRIGGER_NONE |
>>>>>> IRQF_SHARED,
>>>>>> "baycom_ser_fdx", dev)) {
>>>>>> release_region(dev->base_addr, SER12_EXTENT);
>>>>>> return -EBUSY;
>>>>>> diff --git a/drivers/net/hamradio/baycom_ser_hdx.c
>>>>>> b/drivers/net/hamradio/baycom_ser_hdx.c
>>>>>> index e349d86..d91c1fd 100644
>>>>>> --- a/drivers/net/hamradio/baycom_ser_hdx.c
>>>>>> +++ b/drivers/net/hamradio/baycom_ser_hdx.c
>>>>>> @@ -490,7 +490,7 @@ static int ser12_open(struct net_device *dev)
>>>>>> outb(0, FCR(dev->base_addr)); /* disable FIFOs */
>>>>>> outb(0x0d, MCR(dev->base_addr));
>>>>>> outb(0, IER(dev->base_addr));
>>>>>> - if (request_irq(dev->irq, ser12_interrupt, IRQF_DISABLED |
>>>>>> IRQF_SHARED,
>>>>>> + if (request_irq(dev->irq, ser12_interrupt, IRQF_TRIGGER_NONE |
>>>>>> IRQF_SHARED,
>>>>>> "baycom_ser12", dev)) {
>>>>>> release_region(dev->base_addr, SER12_EXTENT);
>>>>>> return -EBUSY;
>>>>>> diff --git a/drivers/net/hamradio/scc.c b/drivers/net/hamradio/scc.c
>>>>>> index bc1d521..4bc6ee8 100644
>>>>>> --- a/drivers/net/hamradio/scc.c
>>>>>> +++ b/drivers/net/hamradio/scc.c
>>>>>> @@ -1734,7 +1734,7 @@ static int scc_net_ioctl(struct net_device *dev,
>>>>>> struct ifreq *ifr, int cmd)
>>>>>> if (!Ivec[hwcfg.irq].used && hwcfg.irq)
>>>>>> {
>>>>>> if (request_irq(hwcfg.irq, scc_isr,
>>>>>> - IRQF_DISABLED, "AX.25 SCC",
>>>>>> + 0, "AX.25 SCC",
>>>>>> (void *)(long) hwcfg.irq))
>>>>>> printk(KERN_WARNING "z8530drv:
>>>>>> warning, cannot get IRQ %d\n", hwcfg.irq);
>>>>>> else
>>>>>> diff --git a/drivers/net/hamradio/yam.c b/drivers/net/hamradio/yam.c
>>>>>> index 0721e72..f947887 100644
>>>>>> --- a/drivers/net/hamradio/yam.c
>>>>>> +++ b/drivers/net/hamradio/yam.c
>>>>>> @@ -888,7 +888,7 @@ static int yam_open(struct net_device *dev)
>>>>>> goto out_release_base;
>>>>>> }
>>>>>> outb(0, IER(dev->base_addr));
>>>>>> - if (request_irq(dev->irq, yam_interrupt, IRQF_DISABLED |
>>>>>> IRQF_SHARED,
>>>>>> dev->name, dev)) {
>>>>>> + if (request_irq(dev->irq, yam_interrupt, IRQF_TRIGGER_NONE |
>>>>>> IRQF_SHARED, dev->name, dev)) {
>>>>>> printk(KERN_ERR "%s: irq %d busy\n", dev->name, dev->irq);
>>>>>> ret = -EBUSY;
>>>>>> goto out_release_base;
>>>>>> --
>>>>>> 1.7.9.5
>>>>>>
>>>>>> --
>>>>>> To unsubscribe from this list: send the line "unsubscribe
>>>>>> kernel-janitors"
>>>>>> in
>>>>>> the body of a message to majordomo@vger.kernel.org
>>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>>>
>>>> Yes i was suggested for using IRQF_TRIGGER_NONE too so used this.
>>>> Please tell
>>>> me if this makes any difference or issue?
>>> I think that the suggestion was to use IRQF_TRIGGER_NONE only when
>>> removing IRQF_DISABLED would leave nothing left.
>>>
>>> - IRQF_DISABLED | e
>>> + e
>>>
>>> otherwise,
>>>
>>> - IRQF_DISABLED
>>> + 0
>>>
>>> julia
>> Sorry to prompt back but i'm confused. if removing IRQF_DISABLED leaves
>> nothing then should i use 0 or IRQF_TRIGGER_NONE?
>> I'm convinced with
>>
>> - IRQF_DISABLED | e
>> + e
>>
>> Regards
>>
>> Kumar Gaurav
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-hams" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] drivers:net:hamradio: Removing Depricated IRQF_DISABLED
From: walter harms @ 2013-08-10 18:49 UTC (permalink / raw)
To: Kumar Gaurav
Cc: Julia Lawall, t.sailer, jreuter, jpr, linux-hams, netdev,
linux-kernel, kernel-janitors
In-Reply-To: <52068329.4010400@gmail.com>
Hello Kumar,
i will try to resolve the riddle ...
doing something like fn(a,b,c,0) is a bad habit.
0 is called a "magic number" because noone reading the code has an idea
why it is there. Using the name IRQF_TRIGGER_NONE good.
Having a named bit like IRQ_TRIGGER|IRQ_NMI is most times also a good idea
because it correspond (most times) to the naming of the bits in the data sheet.
The problems starts when this hits reality since nothing is clearly black or white.
In this case when you remove IRQF_DISABLED disables "bit" something should be left.
a|b|IRQF_DISABLED -> a|b (that is the easy part)
but
IRQF_DISABLED -> IRQF_TRIGGER_NONE
Otherwise you would be left with "0". That is a magic number (see above).
So this is to be replaced by a placeholder called IRQF_TRIGGER_NONE.
hope that really helps and that i have understood the discussion.
re,
wh
Am 10.08.2013 20:15, schrieb Kumar Gaurav:
> On Saturday 10 August 2013 10:54 PM, Julia Lawall wrote:
>> On Sat, 10 Aug 2013, Kumar Gaurav wrote:
>>
>>> On Saturday 10 August 2013 10:17 PM, Julia Lawall wrote:
>>>> If it is in a | with something else, I think you can just remove it.
>>>>
>>>> julia
>>>>
>>>> On Sat, 10 Aug 2013, Kumar Gaurav wrote:
>>>>
>>>>> Removed IRQF_DISABLED as it's deprecated and should be removed
>>>>>
>>>>> Signed-off-by: Kumar Gaurav <kumargauravgupta3@gmail.com>
>>>>> ---
>>>>> drivers/net/hamradio/baycom_ser_fdx.c | 2 +-
>>>>> drivers/net/hamradio/baycom_ser_hdx.c | 2 +-
>>>>> drivers/net/hamradio/scc.c | 2 +-
>>>>> drivers/net/hamradio/yam.c | 2 +-
>>>>> 4 files changed, 4 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/hamradio/baycom_ser_fdx.c
>>>>> b/drivers/net/hamradio/baycom_ser_fdx.c
>>>>> index a974727..c114009 100644
>>>>> --- a/drivers/net/hamradio/baycom_ser_fdx.c
>>>>> +++ b/drivers/net/hamradio/baycom_ser_fdx.c
>>>>> @@ -445,7 +445,7 @@ static int ser12_open(struct net_device *dev)
>>>>> outb(0, FCR(dev->base_addr)); /* disable FIFOs */
>>>>> outb(0x0d, MCR(dev->base_addr));
>>>>> outb(0, IER(dev->base_addr));
>>>>> - if (request_irq(dev->irq, ser12_interrupt, IRQF_DISABLED |
>>>>> IRQF_SHARED,
>>>>> + if (request_irq(dev->irq, ser12_interrupt, IRQF_TRIGGER_NONE |
>>>>> IRQF_SHARED,
>>>>> "baycom_ser_fdx", dev)) {
>>>>> release_region(dev->base_addr, SER12_EXTENT);
>>>>> return -EBUSY;
>>>>> diff --git a/drivers/net/hamradio/baycom_ser_hdx.c
>>>>> b/drivers/net/hamradio/baycom_ser_hdx.c
>>>>> index e349d86..d91c1fd 100644
>>>>> --- a/drivers/net/hamradio/baycom_ser_hdx.c
>>>>> +++ b/drivers/net/hamradio/baycom_ser_hdx.c
>>>>> @@ -490,7 +490,7 @@ static int ser12_open(struct net_device *dev)
>>>>> outb(0, FCR(dev->base_addr)); /* disable FIFOs */
>>>>> outb(0x0d, MCR(dev->base_addr));
>>>>> outb(0, IER(dev->base_addr));
>>>>> - if (request_irq(dev->irq, ser12_interrupt, IRQF_DISABLED |
>>>>> IRQF_SHARED,
>>>>> + if (request_irq(dev->irq, ser12_interrupt, IRQF_TRIGGER_NONE |
>>>>> IRQF_SHARED,
>>>>> "baycom_ser12", dev)) {
>>>>> release_region(dev->base_addr, SER12_EXTENT);
>>>>> return -EBUSY;
>>>>> diff --git a/drivers/net/hamradio/scc.c b/drivers/net/hamradio/scc.c
>>>>> index bc1d521..4bc6ee8 100644
>>>>> --- a/drivers/net/hamradio/scc.c
>>>>> +++ b/drivers/net/hamradio/scc.c
>>>>> @@ -1734,7 +1734,7 @@ static int scc_net_ioctl(struct net_device *dev,
>>>>> struct ifreq *ifr, int cmd)
>>>>> if (!Ivec[hwcfg.irq].used && hwcfg.irq)
>>>>> {
>>>>> if (request_irq(hwcfg.irq, scc_isr,
>>>>> - IRQF_DISABLED, "AX.25 SCC",
>>>>> + 0, "AX.25 SCC",
>>>>> (void *)(long) hwcfg.irq))
>>>>> printk(KERN_WARNING "z8530drv:
>>>>> warning, cannot get IRQ %d\n", hwcfg.irq);
>>>>> else
>>>>> diff --git a/drivers/net/hamradio/yam.c b/drivers/net/hamradio/yam.c
>>>>> index 0721e72..f947887 100644
>>>>> --- a/drivers/net/hamradio/yam.c
>>>>> +++ b/drivers/net/hamradio/yam.c
>>>>> @@ -888,7 +888,7 @@ static int yam_open(struct net_device *dev)
>>>>> goto out_release_base;
>>>>> }
>>>>> outb(0, IER(dev->base_addr));
>>>>> - if (request_irq(dev->irq, yam_interrupt, IRQF_DISABLED |
>>>>> IRQF_SHARED,
>>>>> dev->name, dev)) {
>>>>> + if (request_irq(dev->irq, yam_interrupt, IRQF_TRIGGER_NONE |
>>>>> IRQF_SHARED, dev->name, dev)) {
>>>>> printk(KERN_ERR "%s: irq %d busy\n", dev->name, dev->irq);
>>>>> ret = -EBUSY;
>>>>> goto out_release_base;
>>>>> --
>>>>> 1.7.9.5
>>>>>
>>>>> --
>>>>> To unsubscribe from this list: send the line "unsubscribe
>>>>> kernel-janitors"
>>>>> in
>>>>> the body of a message to majordomo@vger.kernel.org
>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>>
>>> Yes i was suggested for using IRQF_TRIGGER_NONE too so used this.
>>> Please tell
>>> me if this makes any difference or issue?
>> I think that the suggestion was to use IRQF_TRIGGER_NONE only when
>> removing IRQF_DISABLED would leave nothing left.
>>
>> - IRQF_DISABLED | e
>> + e
>>
>> otherwise,
>>
>> - IRQF_DISABLED
>> + 0
>>
>> julia
> Sorry to prompt back but i'm confused. if removing IRQF_DISABLED leaves
> nothing then should i use 0 or IRQF_TRIGGER_NONE?
> I'm convinced with
>
> - IRQF_DISABLED | e
> + e
>
> Regards
>
> Kumar Gaurav
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hams" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] drivers:net:hamradio: Removing Depricated IRQF_DISABLED
From: Julia Lawall @ 2013-08-10 18:42 UTC (permalink / raw)
To: Kumar Gaurav
Cc: Julia Lawall, t.sailer, jreuter, jpr, linux-hams, netdev,
linux-kernel, kernel-janitors
In-Reply-To: <52068329.4010400@gmail.com>
On Sat, 10 Aug 2013, Kumar Gaurav wrote:
> On Saturday 10 August 2013 10:54 PM, Julia Lawall wrote:
> > On Sat, 10 Aug 2013, Kumar Gaurav wrote:
> >
> > > On Saturday 10 August 2013 10:17 PM, Julia Lawall wrote:
> > > > If it is in a | with something else, I think you can just remove it.
> > > >
> > > > julia
> > > >
> > > > On Sat, 10 Aug 2013, Kumar Gaurav wrote:
> > > >
> > > > > Removed IRQF_DISABLED as it's deprecated and should be removed
> > > > >
> > > > > Signed-off-by: Kumar Gaurav <kumargauravgupta3@gmail.com>
> > > > > ---
> > > > > drivers/net/hamradio/baycom_ser_fdx.c | 2 +-
> > > > > drivers/net/hamradio/baycom_ser_hdx.c | 2 +-
> > > > > drivers/net/hamradio/scc.c | 2 +-
> > > > > drivers/net/hamradio/yam.c | 2 +-
> > > > > 4 files changed, 4 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/net/hamradio/baycom_ser_fdx.c
> > > > > b/drivers/net/hamradio/baycom_ser_fdx.c
> > > > > index a974727..c114009 100644
> > > > > --- a/drivers/net/hamradio/baycom_ser_fdx.c
> > > > > +++ b/drivers/net/hamradio/baycom_ser_fdx.c
> > > > > @@ -445,7 +445,7 @@ static int ser12_open(struct net_device *dev)
> > > > > outb(0, FCR(dev->base_addr)); /* disable FIFOs */
> > > > > outb(0x0d, MCR(dev->base_addr));
> > > > > outb(0, IER(dev->base_addr));
> > > > > - if (request_irq(dev->irq, ser12_interrupt, IRQF_DISABLED |
> > > > > IRQF_SHARED,
> > > > > + if (request_irq(dev->irq, ser12_interrupt, IRQF_TRIGGER_NONE |
> > > > > IRQF_SHARED,
> > > > > "baycom_ser_fdx", dev)) {
> > > > > release_region(dev->base_addr, SER12_EXTENT);
> > > > > return -EBUSY;
> > > > > diff --git a/drivers/net/hamradio/baycom_ser_hdx.c
> > > > > b/drivers/net/hamradio/baycom_ser_hdx.c
> > > > > index e349d86..d91c1fd 100644
> > > > > --- a/drivers/net/hamradio/baycom_ser_hdx.c
> > > > > +++ b/drivers/net/hamradio/baycom_ser_hdx.c
> > > > > @@ -490,7 +490,7 @@ static int ser12_open(struct net_device *dev)
> > > > > outb(0, FCR(dev->base_addr)); /* disable FIFOs */
> > > > > outb(0x0d, MCR(dev->base_addr));
> > > > > outb(0, IER(dev->base_addr));
> > > > > - if (request_irq(dev->irq, ser12_interrupt, IRQF_DISABLED |
> > > > > IRQF_SHARED,
> > > > > + if (request_irq(dev->irq, ser12_interrupt, IRQF_TRIGGER_NONE |
> > > > > IRQF_SHARED,
> > > > > "baycom_ser12", dev)) {
> > > > > release_region(dev->base_addr, SER12_EXTENT);
> > > > > return -EBUSY;
> > > > > diff --git a/drivers/net/hamradio/scc.c b/drivers/net/hamradio/scc.c
> > > > > index bc1d521..4bc6ee8 100644
> > > > > --- a/drivers/net/hamradio/scc.c
> > > > > +++ b/drivers/net/hamradio/scc.c
> > > > > @@ -1734,7 +1734,7 @@ static int scc_net_ioctl(struct net_device *dev,
> > > > > struct ifreq *ifr, int cmd)
> > > > > if (!Ivec[hwcfg.irq].used && hwcfg.irq)
> > > > > {
> > > > > if (request_irq(hwcfg.irq, scc_isr,
> > > > > - IRQF_DISABLED, "AX.25
> > > > > SCC",
> > > > > + 0, "AX.25 SCC",
> > > > > (void *)(long)
> > > > > hwcfg.irq))
> > > > > printk(KERN_WARNING "z8530drv:
> > > > > warning, cannot get IRQ %d\n", hwcfg.irq);
> > > > > else
> > > > > diff --git a/drivers/net/hamradio/yam.c b/drivers/net/hamradio/yam.c
> > > > > index 0721e72..f947887 100644
> > > > > --- a/drivers/net/hamradio/yam.c
> > > > > +++ b/drivers/net/hamradio/yam.c
> > > > > @@ -888,7 +888,7 @@ static int yam_open(struct net_device *dev)
> > > > > goto out_release_base;
> > > > > }
> > > > > outb(0, IER(dev->base_addr));
> > > > > - if (request_irq(dev->irq, yam_interrupt, IRQF_DISABLED |
> > > > > IRQF_SHARED,
> > > > > dev->name, dev)) {
> > > > > + if (request_irq(dev->irq, yam_interrupt, IRQF_TRIGGER_NONE |
> > > > > IRQF_SHARED, dev->name, dev)) {
> > > > > printk(KERN_ERR "%s: irq %d busy\n", dev->name,
> > > > > dev->irq);
> > > > > ret = -EBUSY;
> > > > > goto out_release_base;
> > > > > --
> > > > > 1.7.9.5
> > > > >
> > > > > --
> > > > > To unsubscribe from this list: send the line "unsubscribe
> > > > > kernel-janitors"
> > > > > in
> > > > > the body of a message to majordomo@vger.kernel.org
> > > > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > > > >
> > > Yes i was suggested for using IRQF_TRIGGER_NONE too so used this. Please
> > > tell
> > > me if this makes any difference or issue?
> > I think that the suggestion was to use IRQF_TRIGGER_NONE only when
> > removing IRQF_DISABLED would leave nothing left.
> >
> > - IRQF_DISABLED | e
> > + e
> >
> > otherwise,
> >
> > - IRQF_DISABLED
> > + 0
> >
> > julia
> Sorry to prompt back but i'm confused. if removing IRQF_DISABLED leaves
> nothing then should i use 0 or IRQF_TRIGGER_NONE?
I don't know. I think you have to contact some maintainers of affected
code and find out what they prefer.
julia
> I'm convinced with
>
> - IRQF_DISABLED | e
> + e
>
> Regards
>
> Kumar Gaurav
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH] drivers:net:hamradio: Removing Depricated IRQF_DISABLED
From: Kumar Gaurav @ 2013-08-10 18:15 UTC (permalink / raw)
To: Julia Lawall
Cc: t.sailer, jreuter, jpr, linux-hams, netdev, linux-kernel,
kernel-janitors
In-Reply-To: <alpine.DEB.2.02.1308101923130.2076@localhost6.localdomain6>
On Saturday 10 August 2013 10:54 PM, Julia Lawall wrote:
> On Sat, 10 Aug 2013, Kumar Gaurav wrote:
>
>> On Saturday 10 August 2013 10:17 PM, Julia Lawall wrote:
>>> If it is in a | with something else, I think you can just remove it.
>>>
>>> julia
>>>
>>> On Sat, 10 Aug 2013, Kumar Gaurav wrote:
>>>
>>>> Removed IRQF_DISABLED as it's deprecated and should be removed
>>>>
>>>> Signed-off-by: Kumar Gaurav <kumargauravgupta3@gmail.com>
>>>> ---
>>>> drivers/net/hamradio/baycom_ser_fdx.c | 2 +-
>>>> drivers/net/hamradio/baycom_ser_hdx.c | 2 +-
>>>> drivers/net/hamradio/scc.c | 2 +-
>>>> drivers/net/hamradio/yam.c | 2 +-
>>>> 4 files changed, 4 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/net/hamradio/baycom_ser_fdx.c
>>>> b/drivers/net/hamradio/baycom_ser_fdx.c
>>>> index a974727..c114009 100644
>>>> --- a/drivers/net/hamradio/baycom_ser_fdx.c
>>>> +++ b/drivers/net/hamradio/baycom_ser_fdx.c
>>>> @@ -445,7 +445,7 @@ static int ser12_open(struct net_device *dev)
>>>> outb(0, FCR(dev->base_addr)); /* disable FIFOs */
>>>> outb(0x0d, MCR(dev->base_addr));
>>>> outb(0, IER(dev->base_addr));
>>>> - if (request_irq(dev->irq, ser12_interrupt, IRQF_DISABLED |
>>>> IRQF_SHARED,
>>>> + if (request_irq(dev->irq, ser12_interrupt, IRQF_TRIGGER_NONE |
>>>> IRQF_SHARED,
>>>> "baycom_ser_fdx", dev)) {
>>>> release_region(dev->base_addr, SER12_EXTENT);
>>>> return -EBUSY;
>>>> diff --git a/drivers/net/hamradio/baycom_ser_hdx.c
>>>> b/drivers/net/hamradio/baycom_ser_hdx.c
>>>> index e349d86..d91c1fd 100644
>>>> --- a/drivers/net/hamradio/baycom_ser_hdx.c
>>>> +++ b/drivers/net/hamradio/baycom_ser_hdx.c
>>>> @@ -490,7 +490,7 @@ static int ser12_open(struct net_device *dev)
>>>> outb(0, FCR(dev->base_addr)); /* disable FIFOs */
>>>> outb(0x0d, MCR(dev->base_addr));
>>>> outb(0, IER(dev->base_addr));
>>>> - if (request_irq(dev->irq, ser12_interrupt, IRQF_DISABLED |
>>>> IRQF_SHARED,
>>>> + if (request_irq(dev->irq, ser12_interrupt, IRQF_TRIGGER_NONE |
>>>> IRQF_SHARED,
>>>> "baycom_ser12", dev)) {
>>>> release_region(dev->base_addr, SER12_EXTENT);
>>>> return -EBUSY;
>>>> diff --git a/drivers/net/hamradio/scc.c b/drivers/net/hamradio/scc.c
>>>> index bc1d521..4bc6ee8 100644
>>>> --- a/drivers/net/hamradio/scc.c
>>>> +++ b/drivers/net/hamradio/scc.c
>>>> @@ -1734,7 +1734,7 @@ static int scc_net_ioctl(struct net_device *dev,
>>>> struct ifreq *ifr, int cmd)
>>>> if (!Ivec[hwcfg.irq].used && hwcfg.irq)
>>>> {
>>>> if (request_irq(hwcfg.irq, scc_isr,
>>>> - IRQF_DISABLED, "AX.25 SCC",
>>>> + 0, "AX.25 SCC",
>>>> (void *)(long) hwcfg.irq))
>>>> printk(KERN_WARNING "z8530drv:
>>>> warning, cannot get IRQ %d\n", hwcfg.irq);
>>>> else
>>>> diff --git a/drivers/net/hamradio/yam.c b/drivers/net/hamradio/yam.c
>>>> index 0721e72..f947887 100644
>>>> --- a/drivers/net/hamradio/yam.c
>>>> +++ b/drivers/net/hamradio/yam.c
>>>> @@ -888,7 +888,7 @@ static int yam_open(struct net_device *dev)
>>>> goto out_release_base;
>>>> }
>>>> outb(0, IER(dev->base_addr));
>>>> - if (request_irq(dev->irq, yam_interrupt, IRQF_DISABLED | IRQF_SHARED,
>>>> dev->name, dev)) {
>>>> + if (request_irq(dev->irq, yam_interrupt, IRQF_TRIGGER_NONE |
>>>> IRQF_SHARED, dev->name, dev)) {
>>>> printk(KERN_ERR "%s: irq %d busy\n", dev->name, dev->irq);
>>>> ret = -EBUSY;
>>>> goto out_release_base;
>>>> --
>>>> 1.7.9.5
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe kernel-janitors"
>>>> in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>
>> Yes i was suggested for using IRQF_TRIGGER_NONE too so used this. Please tell
>> me if this makes any difference or issue?
> I think that the suggestion was to use IRQF_TRIGGER_NONE only when
> removing IRQF_DISABLED would leave nothing left.
>
> - IRQF_DISABLED | e
> + e
>
> otherwise,
>
> - IRQF_DISABLED
> + 0
>
> julia
Sorry to prompt back but i'm confused. if removing IRQF_DISABLED leaves
nothing then should i use 0 or IRQF_TRIGGER_NONE?
I'm convinced with
- IRQF_DISABLED | e
+ e
Regards
Kumar Gaurav
^ permalink raw reply
* Re: [PATCH net] skge: add dma_mapping check
From: Stephen Hemminger @ 2013-08-10 17:41 UTC (permalink / raw)
To: poma; +Cc: David Miller, netdev
In-Reply-To: <52062955.90102@gmail.com>
On Sat, 10 Aug 2013 13:51:49 +0200
poma <pomidorabelisima@gmail.com> wrote:
> On 05.08.2013 03:35, David Miller wrote:
> > From: Stephen Hemminger <stephen@networkplumber.org>
> > Date: Sun, 4 Aug 2013 17:22:34 -0700
> >
> >> This old driver never checked for DMA mapping errors.
> >> Causing splats with the new DMA mapping checks:
> >> WARNING: at lib/dma-debug.c:937 check_unmap+0x47b/0x930()
> >> skge 0000:01:09.0: DMA-API: device driver failed to check map
> >>
> >> Add checks and unwind code.
> >>
> >> Reported-by: poma <pomidorabelisima@gmail.com>
> >> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> >
> > Applied, but:
> >
> >> -static void skge_rx_setup(struct skge_port *skge, struct skge_element *e,
> >> +static int skge_rx_setup(struct skge_port *skge, struct skge_element *e,
> >> struct sk_buff *skb, unsigned int bufsize)
> >
> > I reflowed the argument indentation for this in the final commit.
> >
> > Thanks.
> >
>
> skge-add-dma_mapping-check.patch
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/patch/drivers/net/ethernet/marvell?id=136d8f377e1575463b47840bc5f1b22d94bf8f63
>
> /usr/lib/modules/3.11.0-0.rc4.git2.1.fc20.x86_64/updates/skge.ko
>
> skge: module verification failed: signature and/or required key missing
> - tainting kernel
> skge: 1.14 addr 0xfbffc000 irq 19 chip Yukon rev 1
> skge 0000:01:09.0 eth0: addr nn:nn:nn:nn:nn:nn
> skge 0000:01:09.0 enp1s9: enabling interface
> skge 0000:01:09.0 enp1s9: Link is up at 1000 Mbps, full duplex, flow
> control both
> IPv6: ADDRCONF(NETDEV_CHANGE): enp1s9: link becomes ready
>
> Starting Nmap 6.25
>
> ------------[ cut here ]------------
> WARNING: CPU: 2 PID: 2443 at lib/dma-debug.c:986 check_sync+0x4bc/0x580()
> skge 0000:01:09.0: DMA-API: device driver tries to sync DMA memory it
> has not allocated [device address=0x00000000cf390040] [size=60 bytes]
> Modules linked in: skge(OF) raid1 nouveau video mxm_wmi i2c_algo_bit
> drm_kms_helper ttm drm i2c_core wmi
> CPU: 2 PID: 2443 Comm: nmap Tainted: GF O
> 3.11.0-0.rc4.git2.1.fc20.x86_64 #1
> Hardware name: Gigabyte Technology Co., Ltd. M720-US3/M720-US3, BIOS F7n
> 09/07/2010
> 0000000000000009 ffff88012aa03c48 ffffffff81723ac3 ffff88012aa03c90
> ffff88012aa03c80 ffffffff8107462d ffff88012837c2b0 000000000000003c
> ffff880128363470 00000000cf390040 ffff880126f79160 ffff88012aa03ce0
> Call Trace:
> <IRQ> [<ffffffff81723ac3>] dump_stack+0x54/0x74
> [<ffffffff8107462d>] warn_slowpath_common+0x7d/0xa0
> [<ffffffff8107469c>] warn_slowpath_fmt+0x4c/0x50
> [<ffffffff81392a5c>] check_sync+0x4bc/0x580
> [<ffffffff8138519e>] ? debug_check_no_obj_freed+0x14e/0x250
> [<ffffffff81392b6b>] debug_dma_sync_single_for_cpu+0x4b/0x50
> [<ffffffff810e6b6c>] ? trace_hardirqs_on_caller+0xac/0x1c0
> [<ffffffff815d9390>] ? build_skb+0x30/0x1d0
> [<ffffffff815db349>] ? __netdev_alloc_skb+0x89/0xf0
> [<ffffffffa01c7e21>] skge_poll+0x3a1/0x9f0 [skge]
> [<ffffffff815edb41>] ? net_rx_action+0xa1/0x380
> [<ffffffff815edc12>] net_rx_action+0x172/0x380
> [<ffffffff8107b4a7>] __do_softirq+0x107/0x410
> [<ffffffff8107b985>] irq_exit+0xc5/0xd0
> [<ffffffff81738c16>] do_IRQ+0x56/0xc0
> [<ffffffff8172d432>] common_interrupt+0x72/0x72
> <EOI> [<ffffffff81721f92>] ? __slab_alloc+0x4c2/0x526
> [<ffffffff810e6bbd>] ? trace_hardirqs_on_caller+0xfd/0x1c0
> [<ffffffff815db08e>] ? __alloc_skb+0x7e/0x2b0
> [<ffffffff811d2d71>] __kmalloc_node_track_caller+0x1a1/0x410
> [<ffffffff815db08e>] ? __alloc_skb+0x7e/0x2b0
> [<ffffffff815da8a1>] __kmalloc_reserve.isra.25+0x31/0x90
> [<ffffffff815db08e>] __alloc_skb+0x7e/0x2b0
> [<ffffffff815d754e>] sock_alloc_send_pskb+0x27e/0x400
> [<ffffffff815d76e5>] sock_alloc_send_skb+0x15/0x20
> [<ffffffff816614af>] raw_sendmsg+0x74f/0xc50
> [<ffffffff81660e7d>] ? raw_sendmsg+0x11d/0xc50
> [<ffffffff8130246d>] ? avc_has_perm_flags+0x16d/0x350
> [<ffffffff81302329>] ? avc_has_perm_flags+0x29/0x350
> [<ffffffff810b797f>] ? local_clock+0x5f/0x70
> [<ffffffff810e3fcf>] ? lock_release_holdtime.part.28+0xf/0x1a0
> [<ffffffff816723a7>] inet_sendmsg+0x117/0x230
> [<ffffffff81672295>] ? inet_sendmsg+0x5/0x230
> [<ffffffff815d0939>] sock_sendmsg+0x99/0xd0
> [<ffffffff810e346d>] ? trace_hardirqs_off+0xd/0x10
> [<ffffffff810e9738>] ? lock_release_non_nested+0x308/0x350
> [<ffffffff811312e7>] ? rcu_irq_exit+0x77/0xc0
> [<ffffffff8172d4f3>] ? retint_restore_args+0x13/0x13
> [<ffffffff815d0e94>] SYSC_sendto+0x124/0x1d0
> [<ffffffff817369c5>] ? sysret_check+0x22/0x5d
> [<ffffffff810e6bbd>] ? trace_hardirqs_on_caller+0xfd/0x1c0
> [<ffffffff8137af2e>] ? trace_hardirqs_on_thunk+0x3a/0x3f
> [<ffffffff815d1ffe>] SyS_sendto+0xe/0x10
> [<ffffffff81736999>] system_call_fastpath+0x16/0x1b
> ---[ end trace ef4521ca4028fd28 ]---
The DMA debug check insists that the call to sync_single has
to be the same length as the mapping. The driver is only bothering
to sync the bytes that matter (got received) rather than the whole
map.
In reality that is not required, but I will make a patch anyway.
^ permalink raw reply
* Re: [PATCH] drivers:net:hamradio: Removing Depricated IRQF_DISABLED
From: Julia Lawall @ 2013-08-10 17:24 UTC (permalink / raw)
To: Kumar Gaurav
Cc: Julia Lawall, t.sailer, jreuter, jpr, linux-hams, netdev,
linux-kernel, kernel-janitors
In-Reply-To: <5206720C.4060405@gmail.com>
On Sat, 10 Aug 2013, Kumar Gaurav wrote:
> On Saturday 10 August 2013 10:17 PM, Julia Lawall wrote:
> > If it is in a | with something else, I think you can just remove it.
> >
> > julia
> >
> > On Sat, 10 Aug 2013, Kumar Gaurav wrote:
> >
> > > Removed IRQF_DISABLED as it's deprecated and should be removed
> > >
> > > Signed-off-by: Kumar Gaurav <kumargauravgupta3@gmail.com>
> > > ---
> > > drivers/net/hamradio/baycom_ser_fdx.c | 2 +-
> > > drivers/net/hamradio/baycom_ser_hdx.c | 2 +-
> > > drivers/net/hamradio/scc.c | 2 +-
> > > drivers/net/hamradio/yam.c | 2 +-
> > > 4 files changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/net/hamradio/baycom_ser_fdx.c
> > > b/drivers/net/hamradio/baycom_ser_fdx.c
> > > index a974727..c114009 100644
> > > --- a/drivers/net/hamradio/baycom_ser_fdx.c
> > > +++ b/drivers/net/hamradio/baycom_ser_fdx.c
> > > @@ -445,7 +445,7 @@ static int ser12_open(struct net_device *dev)
> > > outb(0, FCR(dev->base_addr)); /* disable FIFOs */
> > > outb(0x0d, MCR(dev->base_addr));
> > > outb(0, IER(dev->base_addr));
> > > - if (request_irq(dev->irq, ser12_interrupt, IRQF_DISABLED |
> > > IRQF_SHARED,
> > > + if (request_irq(dev->irq, ser12_interrupt, IRQF_TRIGGER_NONE |
> > > IRQF_SHARED,
> > > "baycom_ser_fdx", dev)) {
> > > release_region(dev->base_addr, SER12_EXTENT);
> > > return -EBUSY;
> > > diff --git a/drivers/net/hamradio/baycom_ser_hdx.c
> > > b/drivers/net/hamradio/baycom_ser_hdx.c
> > > index e349d86..d91c1fd 100644
> > > --- a/drivers/net/hamradio/baycom_ser_hdx.c
> > > +++ b/drivers/net/hamradio/baycom_ser_hdx.c
> > > @@ -490,7 +490,7 @@ static int ser12_open(struct net_device *dev)
> > > outb(0, FCR(dev->base_addr)); /* disable FIFOs */
> > > outb(0x0d, MCR(dev->base_addr));
> > > outb(0, IER(dev->base_addr));
> > > - if (request_irq(dev->irq, ser12_interrupt, IRQF_DISABLED |
> > > IRQF_SHARED,
> > > + if (request_irq(dev->irq, ser12_interrupt, IRQF_TRIGGER_NONE |
> > > IRQF_SHARED,
> > > "baycom_ser12", dev)) {
> > > release_region(dev->base_addr, SER12_EXTENT);
> > > return -EBUSY;
> > > diff --git a/drivers/net/hamradio/scc.c b/drivers/net/hamradio/scc.c
> > > index bc1d521..4bc6ee8 100644
> > > --- a/drivers/net/hamradio/scc.c
> > > +++ b/drivers/net/hamradio/scc.c
> > > @@ -1734,7 +1734,7 @@ static int scc_net_ioctl(struct net_device *dev,
> > > struct ifreq *ifr, int cmd)
> > > if (!Ivec[hwcfg.irq].used && hwcfg.irq)
> > > {
> > > if (request_irq(hwcfg.irq, scc_isr,
> > > - IRQF_DISABLED, "AX.25 SCC",
> > > + 0, "AX.25 SCC",
> > > (void *)(long) hwcfg.irq))
> > > printk(KERN_WARNING "z8530drv:
> > > warning, cannot get IRQ %d\n", hwcfg.irq);
> > > else
> > > diff --git a/drivers/net/hamradio/yam.c b/drivers/net/hamradio/yam.c
> > > index 0721e72..f947887 100644
> > > --- a/drivers/net/hamradio/yam.c
> > > +++ b/drivers/net/hamradio/yam.c
> > > @@ -888,7 +888,7 @@ static int yam_open(struct net_device *dev)
> > > goto out_release_base;
> > > }
> > > outb(0, IER(dev->base_addr));
> > > - if (request_irq(dev->irq, yam_interrupt, IRQF_DISABLED | IRQF_SHARED,
> > > dev->name, dev)) {
> > > + if (request_irq(dev->irq, yam_interrupt, IRQF_TRIGGER_NONE |
> > > IRQF_SHARED, dev->name, dev)) {
> > > printk(KERN_ERR "%s: irq %d busy\n", dev->name, dev->irq);
> > > ret = -EBUSY;
> > > goto out_release_base;
> > > --
> > > 1.7.9.5
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe kernel-janitors"
> > > in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > >
> Yes i was suggested for using IRQF_TRIGGER_NONE too so used this. Please tell
> me if this makes any difference or issue?
I think that the suggestion was to use IRQF_TRIGGER_NONE only when
removing IRQF_DISABLED would leave nothing left.
- IRQF_DISABLED | e
+ e
otherwise,
- IRQF_DISABLED
+ 0
julia
^ permalink raw reply
* Re: [PATCH] drivers:net:hamradio: Removing Depricated IRQF_DISABLED
From: Kumar Gaurav @ 2013-08-10 17:02 UTC (permalink / raw)
To: Julia Lawall
Cc: t.sailer, jreuter, jpr, linux-hams, netdev, linux-kernel,
kernel-janitors
In-Reply-To: <alpine.DEB.2.02.1308101847260.2076@localhost6.localdomain6>
On Saturday 10 August 2013 10:17 PM, Julia Lawall wrote:
> If it is in a | with something else, I think you can just remove it.
>
> julia
>
> On Sat, 10 Aug 2013, Kumar Gaurav wrote:
>
>> Removed IRQF_DISABLED as it's deprecated and should be removed
>>
>> Signed-off-by: Kumar Gaurav <kumargauravgupta3@gmail.com>
>> ---
>> drivers/net/hamradio/baycom_ser_fdx.c | 2 +-
>> drivers/net/hamradio/baycom_ser_hdx.c | 2 +-
>> drivers/net/hamradio/scc.c | 2 +-
>> drivers/net/hamradio/yam.c | 2 +-
>> 4 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/hamradio/baycom_ser_fdx.c b/drivers/net/hamradio/baycom_ser_fdx.c
>> index a974727..c114009 100644
>> --- a/drivers/net/hamradio/baycom_ser_fdx.c
>> +++ b/drivers/net/hamradio/baycom_ser_fdx.c
>> @@ -445,7 +445,7 @@ static int ser12_open(struct net_device *dev)
>> outb(0, FCR(dev->base_addr)); /* disable FIFOs */
>> outb(0x0d, MCR(dev->base_addr));
>> outb(0, IER(dev->base_addr));
>> - if (request_irq(dev->irq, ser12_interrupt, IRQF_DISABLED | IRQF_SHARED,
>> + if (request_irq(dev->irq, ser12_interrupt, IRQF_TRIGGER_NONE | IRQF_SHARED,
>> "baycom_ser_fdx", dev)) {
>> release_region(dev->base_addr, SER12_EXTENT);
>> return -EBUSY;
>> diff --git a/drivers/net/hamradio/baycom_ser_hdx.c b/drivers/net/hamradio/baycom_ser_hdx.c
>> index e349d86..d91c1fd 100644
>> --- a/drivers/net/hamradio/baycom_ser_hdx.c
>> +++ b/drivers/net/hamradio/baycom_ser_hdx.c
>> @@ -490,7 +490,7 @@ static int ser12_open(struct net_device *dev)
>> outb(0, FCR(dev->base_addr)); /* disable FIFOs */
>> outb(0x0d, MCR(dev->base_addr));
>> outb(0, IER(dev->base_addr));
>> - if (request_irq(dev->irq, ser12_interrupt, IRQF_DISABLED | IRQF_SHARED,
>> + if (request_irq(dev->irq, ser12_interrupt, IRQF_TRIGGER_NONE | IRQF_SHARED,
>> "baycom_ser12", dev)) {
>> release_region(dev->base_addr, SER12_EXTENT);
>> return -EBUSY;
>> diff --git a/drivers/net/hamradio/scc.c b/drivers/net/hamradio/scc.c
>> index bc1d521..4bc6ee8 100644
>> --- a/drivers/net/hamradio/scc.c
>> +++ b/drivers/net/hamradio/scc.c
>> @@ -1734,7 +1734,7 @@ static int scc_net_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
>> if (!Ivec[hwcfg.irq].used && hwcfg.irq)
>> {
>> if (request_irq(hwcfg.irq, scc_isr,
>> - IRQF_DISABLED, "AX.25 SCC",
>> + 0, "AX.25 SCC",
>> (void *)(long) hwcfg.irq))
>> printk(KERN_WARNING "z8530drv: warning, cannot get IRQ %d\n", hwcfg.irq);
>> else
>> diff --git a/drivers/net/hamradio/yam.c b/drivers/net/hamradio/yam.c
>> index 0721e72..f947887 100644
>> --- a/drivers/net/hamradio/yam.c
>> +++ b/drivers/net/hamradio/yam.c
>> @@ -888,7 +888,7 @@ static int yam_open(struct net_device *dev)
>> goto out_release_base;
>> }
>> outb(0, IER(dev->base_addr));
>> - if (request_irq(dev->irq, yam_interrupt, IRQF_DISABLED | IRQF_SHARED, dev->name, dev)) {
>> + if (request_irq(dev->irq, yam_interrupt, IRQF_TRIGGER_NONE | IRQF_SHARED, dev->name, dev)) {
>> printk(KERN_ERR "%s: irq %d busy\n", dev->name, dev->irq);
>> ret = -EBUSY;
>> goto out_release_base;
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
Yes i was suggested for using IRQF_TRIGGER_NONE too so used this. Please
tell me if this makes any difference or issue?
Regards
Kumar Gaurav
^ permalink raw reply
* [PATCH 4/4] netfilter: nf_conntrack: fix tcp_in_window for Fast Open
From: Pablo Neira Ayuso @ 2013-08-10 16:58 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1376153933-5747-1-git-send-email-pablo@netfilter.org>
From: Yuchung Cheng <ycheng@google.com>
Currently the conntrack checks if the ending sequence of a packet
falls within the observed receive window. However it does so even
if it has not observe any packet from the remote yet and uses an
uninitialized receive window (td_maxwin).
If a connection uses Fast Open to send a SYN-data packet which is
dropped afterward in the network. The subsequent SYNs retransmits
will all fail this check and be discarded, leading to a connection
timeout. This is because the SYN retransmit does not contain data
payload so
end == initial sequence number (isn) + 1
sender->td_end == isn + syn_data_len
receiver->td_maxwin == 0
The fix is to only apply this check after td_maxwin is initialized.
Reported-by: Michael Chan <mcfchan@stanford.edu>
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Acked-by: Eric Dumazet <edumazet@google.com>
Acked-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nf_conntrack_proto_tcp.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
index 7dcc376..2f80107 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -526,7 +526,7 @@ static bool tcp_in_window(const struct nf_conn *ct,
const struct nf_conntrack_tuple *tuple = &ct->tuplehash[dir].tuple;
__u32 seq, ack, sack, end, win, swin;
s16 receiver_offset;
- bool res;
+ bool res, in_recv_win;
/*
* Get the required data from the packet.
@@ -649,14 +649,18 @@ static bool tcp_in_window(const struct nf_conn *ct,
receiver->td_end, receiver->td_maxend, receiver->td_maxwin,
receiver->td_scale);
+ /* Is the ending sequence in the receive window (if available)? */
+ in_recv_win = !receiver->td_maxwin ||
+ after(end, sender->td_end - receiver->td_maxwin - 1);
+
pr_debug("tcp_in_window: I=%i II=%i III=%i IV=%i\n",
before(seq, sender->td_maxend + 1),
- after(end, sender->td_end - receiver->td_maxwin - 1),
+ (in_recv_win ? 1 : 0),
before(sack, receiver->td_end + 1),
after(sack, receiver->td_end - MAXACKWINDOW(sender) - 1));
if (before(seq, sender->td_maxend + 1) &&
- after(end, sender->td_end - receiver->td_maxwin - 1) &&
+ in_recv_win &&
before(sack, receiver->td_end + 1) &&
after(sack, receiver->td_end - MAXACKWINDOW(sender) - 1)) {
/*
@@ -725,7 +729,7 @@ static bool tcp_in_window(const struct nf_conn *ct,
nf_log_packet(net, pf, 0, skb, NULL, NULL, NULL,
"nf_ct_tcp: %s ",
before(seq, sender->td_maxend + 1) ?
- after(end, sender->td_end - receiver->td_maxwin - 1) ?
+ in_recv_win ?
before(sack, receiver->td_end + 1) ?
after(sack, receiver->td_end - MAXACKWINDOW(sender) - 1) ? "BUG"
: "ACK is under the lower bound (possible overly delayed ACK)"
--
1.7.10.4
^ permalink raw reply related
* [PATCH 3/4] netfilter: nfnetlink_{log,queue}: fix information leaks in netlink message
From: Pablo Neira Ayuso @ 2013-08-10 16:58 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1376153933-5747-1-git-send-email-pablo@netfilter.org>
From: Dan Carpenter <dan.carpenter@oracle.com>
These structs have a "_pad" member. Also the "phw" structs have an 8
byte "hw_addr[]" array but sometimes only the first 6 bytes are
initialized.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nfnetlink_log.c | 6 +++++-
net/netfilter/nfnetlink_queue_core.c | 5 ++++-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index 962e979..d92cc31 100644
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -419,6 +419,7 @@ __build_packet_message(struct nfnl_log_net *log,
nfmsg->version = NFNETLINK_V0;
nfmsg->res_id = htons(inst->group_num);
+ memset(&pmsg, 0, sizeof(pmsg));
pmsg.hw_protocol = skb->protocol;
pmsg.hook = hooknum;
@@ -498,7 +499,10 @@ __build_packet_message(struct nfnl_log_net *log,
if (indev && skb->dev &&
skb->mac_header != skb->network_header) {
struct nfulnl_msg_packet_hw phw;
- int len = dev_parse_header(skb, phw.hw_addr);
+ int len;
+
+ memset(&phw, 0, sizeof(phw));
+ len = dev_parse_header(skb, phw.hw_addr);
if (len > 0) {
phw.hw_addrlen = htons(len);
if (nla_put(inst->skb, NFULA_HWADDR, sizeof(phw), &phw))
diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c
index 971ea14..8a703c3 100644
--- a/net/netfilter/nfnetlink_queue_core.c
+++ b/net/netfilter/nfnetlink_queue_core.c
@@ -463,7 +463,10 @@ nfqnl_build_packet_message(struct nfqnl_instance *queue,
if (indev && entskb->dev &&
entskb->mac_header != entskb->network_header) {
struct nfqnl_msg_packet_hw phw;
- int len = dev_parse_header(entskb, phw.hw_addr);
+ int len;
+
+ memset(&phw, 0, sizeof(phw));
+ len = dev_parse_header(entskb, phw.hw_addr);
if (len) {
phw.hw_addrlen = htons(len);
if (nla_put(skb, NFQA_HWADDR, sizeof(phw), &phw))
--
1.7.10.4
^ permalink raw reply related
* [PATCH 2/4] netfilter: xt_TCPOPTSTRIP: fix possible off by one access
From: Pablo Neira Ayuso @ 2013-08-10 16:58 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1376153933-5747-1-git-send-email-pablo@netfilter.org>
Fix a possible off by one access since optlen()
touches opt[offset+1] unsafely when i == tcp_hdrlen(skb) - 1.
This patch replaces tcp_hdrlen() by the local variable tcp_hdrlen
that stores the TCP header length, to save some cycles.
Reported-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/xt_TCPOPTSTRIP.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/netfilter/xt_TCPOPTSTRIP.c b/net/netfilter/xt_TCPOPTSTRIP.c
index b68fa19..625fa1d 100644
--- a/net/netfilter/xt_TCPOPTSTRIP.c
+++ b/net/netfilter/xt_TCPOPTSTRIP.c
@@ -38,7 +38,7 @@ tcpoptstrip_mangle_packet(struct sk_buff *skb,
struct tcphdr *tcph;
u_int16_t n, o;
u_int8_t *opt;
- int len;
+ int len, tcp_hdrlen;
/* This is a fragment, no TCP header is available */
if (par->fragoff != 0)
@@ -52,7 +52,9 @@ tcpoptstrip_mangle_packet(struct sk_buff *skb,
return NF_DROP;
tcph = (struct tcphdr *)(skb_network_header(skb) + tcphoff);
- if (tcph->doff * 4 > len)
+ tcp_hdrlen = tcph->doff * 4;
+
+ if (len < tcp_hdrlen)
return NF_DROP;
opt = (u_int8_t *)tcph;
@@ -61,10 +63,10 @@ tcpoptstrip_mangle_packet(struct sk_buff *skb,
* Walk through all TCP options - if we find some option to remove,
* set all octets to %TCPOPT_NOP and adjust checksum.
*/
- for (i = sizeof(struct tcphdr); i < tcp_hdrlen(skb); i += optl) {
+ for (i = sizeof(struct tcphdr); i < tcp_hdrlen - 1; i += optl) {
optl = optlen(opt, i);
- if (i + optl > tcp_hdrlen(skb))
+ if (i + optl > tcp_hdrlen)
break;
if (!tcpoptstrip_test_bit(info->strip_bmap, opt[i]))
--
1.7.10.4
^ permalink raw reply related
* [PATCH 0/4] netfilter fixes for 3.11-rc4
From: Pablo Neira Ayuso @ 2013-08-10 16:58 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
Hi David,
The following patchset contains four netfilter fixes, they are:
* Fix possible invalid access and mangling of the TCPMSS option in
xt_TCPMSS. This was spotted by Julian Anastasov.
* Fix possible off by one access and mangling of the TCP packet in
xt_TCPOPTSTRIP, also spotted by Julian Anastasov.
* Fix possible information leak due to missing initialization of one
padding field of several structures that are included in nfqueue and
nflog netlink messages, from Dan Carpenter.
* Fix TCP window tracking with Fast Open, from Yuchung Cheng.
You can pull these changes from:
git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git master
Thanks!
----------------------------------------------------------------
The following changes since commit a661b43fd047ef501da43a19975415f861c7c3db:
mlx5: fix error return code in mlx5_alloc_uuars() (2013-07-30 19:33:45 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git master
for you to fetch changes up to 356d7d88e088687b6578ca64601b0a2c9d145296:
netfilter: nf_conntrack: fix tcp_in_window for Fast Open (2013-08-10 18:36:22 +0200)
----------------------------------------------------------------
Dan Carpenter (1):
netfilter: nfnetlink_{log,queue}: fix information leaks in netlink message
Pablo Neira Ayuso (2):
netfilter: xt_TCPMSS: fix handling of malformed TCP header and options
netfilter: xt_TCPOPTSTRIP: fix possible off by one access
Yuchung Cheng (1):
netfilter: nf_conntrack: fix tcp_in_window for Fast Open
net/netfilter/nf_conntrack_proto_tcp.c | 12 ++++++++----
net/netfilter/nfnetlink_log.c | 6 +++++-
net/netfilter/nfnetlink_queue_core.c | 5 ++++-
net/netfilter/xt_TCPMSS.c | 28 ++++++++++++++++------------
net/netfilter/xt_TCPOPTSTRIP.c | 10 ++++++----
5 files changed, 39 insertions(+), 22 deletions(-)
^ permalink raw reply
* [PATCH 1/4] netfilter: xt_TCPMSS: fix handling of malformed TCP header and options
From: Pablo Neira Ayuso @ 2013-08-10 16:58 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1376153933-5747-1-git-send-email-pablo@netfilter.org>
Make sure the packet has enough room for the TCP header and
that it is not malformed.
While at it, store tcph->doff*4 in a variable, as it is used
several times.
This patch also fixes a possible off by one in case of malformed
TCP options.
Reported-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/xt_TCPMSS.c | 28 ++++++++++++++++------------
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/net/netfilter/xt_TCPMSS.c b/net/netfilter/xt_TCPMSS.c
index 7011c71..6113cc7 100644
--- a/net/netfilter/xt_TCPMSS.c
+++ b/net/netfilter/xt_TCPMSS.c
@@ -52,7 +52,8 @@ tcpmss_mangle_packet(struct sk_buff *skb,
{
const struct xt_tcpmss_info *info = par->targinfo;
struct tcphdr *tcph;
- unsigned int tcplen, i;
+ int len, tcp_hdrlen;
+ unsigned int i;
__be16 oldval;
u16 newmss;
u8 *opt;
@@ -64,11 +65,14 @@ tcpmss_mangle_packet(struct sk_buff *skb,
if (!skb_make_writable(skb, skb->len))
return -1;
- tcplen = skb->len - tcphoff;
+ len = skb->len - tcphoff;
+ if (len < (int)sizeof(struct tcphdr))
+ return -1;
+
tcph = (struct tcphdr *)(skb_network_header(skb) + tcphoff);
+ tcp_hdrlen = tcph->doff * 4;
- /* Header cannot be larger than the packet */
- if (tcplen < tcph->doff*4)
+ if (len < tcp_hdrlen)
return -1;
if (info->mss == XT_TCPMSS_CLAMP_PMTU) {
@@ -87,9 +91,8 @@ tcpmss_mangle_packet(struct sk_buff *skb,
newmss = info->mss;
opt = (u_int8_t *)tcph;
- for (i = sizeof(struct tcphdr); i < tcph->doff*4; i += optlen(opt, i)) {
- if (opt[i] == TCPOPT_MSS && tcph->doff*4 - i >= TCPOLEN_MSS &&
- opt[i+1] == TCPOLEN_MSS) {
+ for (i = sizeof(struct tcphdr); i <= tcp_hdrlen - TCPOLEN_MSS; i += optlen(opt, i)) {
+ if (opt[i] == TCPOPT_MSS && opt[i+1] == TCPOLEN_MSS) {
u_int16_t oldmss;
oldmss = (opt[i+2] << 8) | opt[i+3];
@@ -112,9 +115,10 @@ tcpmss_mangle_packet(struct sk_buff *skb,
}
/* There is data after the header so the option can't be added
- without moving it, and doing so may make the SYN packet
- itself too large. Accept the packet unmodified instead. */
- if (tcplen > tcph->doff*4)
+ * without moving it, and doing so may make the SYN packet
+ * itself too large. Accept the packet unmodified instead.
+ */
+ if (len > tcp_hdrlen)
return 0;
/*
@@ -143,10 +147,10 @@ tcpmss_mangle_packet(struct sk_buff *skb,
newmss = min(newmss, (u16)1220);
opt = (u_int8_t *)tcph + sizeof(struct tcphdr);
- memmove(opt + TCPOLEN_MSS, opt, tcplen - sizeof(struct tcphdr));
+ memmove(opt + TCPOLEN_MSS, opt, len - sizeof(struct tcphdr));
inet_proto_csum_replace2(&tcph->check, skb,
- htons(tcplen), htons(tcplen + TCPOLEN_MSS), 1);
+ htons(len), htons(len + TCPOLEN_MSS), 1);
opt[0] = TCPOPT_MSS;
opt[1] = TCPOLEN_MSS;
opt[2] = (newmss & 0xff00) >> 8;
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] drivers:net:hamradio: Removing Depricated IRQF_DISABLED
From: Julia Lawall @ 2013-08-10 16:47 UTC (permalink / raw)
To: Kumar Gaurav
Cc: t.sailer, jreuter, jpr, linux-hams, netdev, linux-kernel,
kernel-janitors
In-Reply-To: <1376152738-14944-1-git-send-email-kumargauravgupta3@gmail.com>
If it is in a | with something else, I think you can just remove it.
julia
On Sat, 10 Aug 2013, Kumar Gaurav wrote:
> Removed IRQF_DISABLED as it's deprecated and should be removed
>
> Signed-off-by: Kumar Gaurav <kumargauravgupta3@gmail.com>
> ---
> drivers/net/hamradio/baycom_ser_fdx.c | 2 +-
> drivers/net/hamradio/baycom_ser_hdx.c | 2 +-
> drivers/net/hamradio/scc.c | 2 +-
> drivers/net/hamradio/yam.c | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/hamradio/baycom_ser_fdx.c b/drivers/net/hamradio/baycom_ser_fdx.c
> index a974727..c114009 100644
> --- a/drivers/net/hamradio/baycom_ser_fdx.c
> +++ b/drivers/net/hamradio/baycom_ser_fdx.c
> @@ -445,7 +445,7 @@ static int ser12_open(struct net_device *dev)
> outb(0, FCR(dev->base_addr)); /* disable FIFOs */
> outb(0x0d, MCR(dev->base_addr));
> outb(0, IER(dev->base_addr));
> - if (request_irq(dev->irq, ser12_interrupt, IRQF_DISABLED | IRQF_SHARED,
> + if (request_irq(dev->irq, ser12_interrupt, IRQF_TRIGGER_NONE | IRQF_SHARED,
> "baycom_ser_fdx", dev)) {
> release_region(dev->base_addr, SER12_EXTENT);
> return -EBUSY;
> diff --git a/drivers/net/hamradio/baycom_ser_hdx.c b/drivers/net/hamradio/baycom_ser_hdx.c
> index e349d86..d91c1fd 100644
> --- a/drivers/net/hamradio/baycom_ser_hdx.c
> +++ b/drivers/net/hamradio/baycom_ser_hdx.c
> @@ -490,7 +490,7 @@ static int ser12_open(struct net_device *dev)
> outb(0, FCR(dev->base_addr)); /* disable FIFOs */
> outb(0x0d, MCR(dev->base_addr));
> outb(0, IER(dev->base_addr));
> - if (request_irq(dev->irq, ser12_interrupt, IRQF_DISABLED | IRQF_SHARED,
> + if (request_irq(dev->irq, ser12_interrupt, IRQF_TRIGGER_NONE | IRQF_SHARED,
> "baycom_ser12", dev)) {
> release_region(dev->base_addr, SER12_EXTENT);
> return -EBUSY;
> diff --git a/drivers/net/hamradio/scc.c b/drivers/net/hamradio/scc.c
> index bc1d521..4bc6ee8 100644
> --- a/drivers/net/hamradio/scc.c
> +++ b/drivers/net/hamradio/scc.c
> @@ -1734,7 +1734,7 @@ static int scc_net_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
> if (!Ivec[hwcfg.irq].used && hwcfg.irq)
> {
> if (request_irq(hwcfg.irq, scc_isr,
> - IRQF_DISABLED, "AX.25 SCC",
> + 0, "AX.25 SCC",
> (void *)(long) hwcfg.irq))
> printk(KERN_WARNING "z8530drv: warning, cannot get IRQ %d\n", hwcfg.irq);
> else
> diff --git a/drivers/net/hamradio/yam.c b/drivers/net/hamradio/yam.c
> index 0721e72..f947887 100644
> --- a/drivers/net/hamradio/yam.c
> +++ b/drivers/net/hamradio/yam.c
> @@ -888,7 +888,7 @@ static int yam_open(struct net_device *dev)
> goto out_release_base;
> }
> outb(0, IER(dev->base_addr));
> - if (request_irq(dev->irq, yam_interrupt, IRQF_DISABLED | IRQF_SHARED, dev->name, dev)) {
> + if (request_irq(dev->irq, yam_interrupt, IRQF_TRIGGER_NONE | IRQF_SHARED, dev->name, dev)) {
> printk(KERN_ERR "%s: irq %d busy\n", dev->name, dev->irq);
> ret = -EBUSY;
> goto out_release_base;
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* [PATCH] drivers:net:hamradio: Removing Depricated IRQF_DISABLED
From: Kumar Gaurav @ 2013-08-10 16:38 UTC (permalink / raw)
To: t.sailer, jreuter, jpr
Cc: linux-hams, netdev, linux-kernel, kernel-janitors, Kumar Gaurav
Removed IRQF_DISABLED as it's deprecated and should be removed
Signed-off-by: Kumar Gaurav <kumargauravgupta3@gmail.com>
---
drivers/net/hamradio/baycom_ser_fdx.c | 2 +-
drivers/net/hamradio/baycom_ser_hdx.c | 2 +-
drivers/net/hamradio/scc.c | 2 +-
drivers/net/hamradio/yam.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/hamradio/baycom_ser_fdx.c b/drivers/net/hamradio/baycom_ser_fdx.c
index a974727..c114009 100644
--- a/drivers/net/hamradio/baycom_ser_fdx.c
+++ b/drivers/net/hamradio/baycom_ser_fdx.c
@@ -445,7 +445,7 @@ static int ser12_open(struct net_device *dev)
outb(0, FCR(dev->base_addr)); /* disable FIFOs */
outb(0x0d, MCR(dev->base_addr));
outb(0, IER(dev->base_addr));
- if (request_irq(dev->irq, ser12_interrupt, IRQF_DISABLED | IRQF_SHARED,
+ if (request_irq(dev->irq, ser12_interrupt, IRQF_TRIGGER_NONE | IRQF_SHARED,
"baycom_ser_fdx", dev)) {
release_region(dev->base_addr, SER12_EXTENT);
return -EBUSY;
diff --git a/drivers/net/hamradio/baycom_ser_hdx.c b/drivers/net/hamradio/baycom_ser_hdx.c
index e349d86..d91c1fd 100644
--- a/drivers/net/hamradio/baycom_ser_hdx.c
+++ b/drivers/net/hamradio/baycom_ser_hdx.c
@@ -490,7 +490,7 @@ static int ser12_open(struct net_device *dev)
outb(0, FCR(dev->base_addr)); /* disable FIFOs */
outb(0x0d, MCR(dev->base_addr));
outb(0, IER(dev->base_addr));
- if (request_irq(dev->irq, ser12_interrupt, IRQF_DISABLED | IRQF_SHARED,
+ if (request_irq(dev->irq, ser12_interrupt, IRQF_TRIGGER_NONE | IRQF_SHARED,
"baycom_ser12", dev)) {
release_region(dev->base_addr, SER12_EXTENT);
return -EBUSY;
diff --git a/drivers/net/hamradio/scc.c b/drivers/net/hamradio/scc.c
index bc1d521..4bc6ee8 100644
--- a/drivers/net/hamradio/scc.c
+++ b/drivers/net/hamradio/scc.c
@@ -1734,7 +1734,7 @@ static int scc_net_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
if (!Ivec[hwcfg.irq].used && hwcfg.irq)
{
if (request_irq(hwcfg.irq, scc_isr,
- IRQF_DISABLED, "AX.25 SCC",
+ 0, "AX.25 SCC",
(void *)(long) hwcfg.irq))
printk(KERN_WARNING "z8530drv: warning, cannot get IRQ %d\n", hwcfg.irq);
else
diff --git a/drivers/net/hamradio/yam.c b/drivers/net/hamradio/yam.c
index 0721e72..f947887 100644
--- a/drivers/net/hamradio/yam.c
+++ b/drivers/net/hamradio/yam.c
@@ -888,7 +888,7 @@ static int yam_open(struct net_device *dev)
goto out_release_base;
}
outb(0, IER(dev->base_addr));
- if (request_irq(dev->irq, yam_interrupt, IRQF_DISABLED | IRQF_SHARED, dev->name, dev)) {
+ if (request_irq(dev->irq, yam_interrupt, IRQF_TRIGGER_NONE | IRQF_SHARED, dev->name, dev)) {
printk(KERN_ERR "%s: irq %d busy\n", dev->name, dev->irq);
ret = -EBUSY;
goto out_release_base;
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] netfilter: nf_conntrack: fix tcp_in_window for Fast Open
From: Pablo Neira Ayuso @ 2013-08-10 16:38 UTC (permalink / raw)
To: Jozsef Kadlecsik; +Cc: Yuchung Cheng, netfilter-devel, netdev, edumazet
In-Reply-To: <alpine.DEB.2.00.1308101501070.5256@blackhole.kfki.hu>
On Sat, Aug 10, 2013 at 03:01:36PM +0200, Jozsef Kadlecsik wrote:
> On Fri, 9 Aug 2013, Yuchung Cheng wrote:
>
> > Currently the conntrack checks if the ending sequence of a packet
> > falls within the observed receive window. However it does so even
> > if it has not observe any packet from the remote yet and uses an
> > uninitialized receive window (td_maxwin).
> >
> > If a connection uses Fast Open to send a SYN-data packet which is
> > dropped afterward in the network. The subsequent SYNs retransmits
> > will all fail this check and be discarded, leading to a connection
> > timeout. This is because the SYN retransmit does not contain data
> > payload so
> >
> > end == initial sequence number (isn) + 1
> > sender->td_end == isn + syn_data_len
> > receiver->td_maxwin == 0
> >
> > The fix is to only apply this check after td_maxwin is initialized.
> >
> > Reported-by: Michael Chan <mcfchan@stanford.edu>
> > Signed-off-by: Yuchung Cheng <ycheng@google.com>
>
> Acked-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Applied, thanks everyone.
^ permalink raw reply
* Re: [PATCH RFC] xfrm{4,6}: only report errors back to local sockets if we don't cross address family
From: Hannes Frederic Sowa @ 2013-08-10 16:16 UTC (permalink / raw)
To: Eric Dumazet, Steffen Klassert, netdev, vi0oss
In-Reply-To: <20130808230620.GK14001@order.stressinduktion.org>
On Fri, Aug 09, 2013 at 01:06:20AM +0200, Hannes Frederic Sowa wrote:
> I will check if the skb->encapsulated bit could help. Actually I don't
> know what the correct behaviour for error reporting should be in the end,
> maybe: dispatch packet back to tunnel interface, let tunnel interface
> decapsulate the inner packet and use this inner header to inform the
> original socket about the error.
Seems skb->encapsulated helps, but I still have to wire it up for the ipv6
tunnels.
I just prototyped this patch, but I fear I now introduced a dependency
from core xfrm to ipv6, which I would like to have prevented (this would
even happen if I put xfrm_local_error in a header file). Is this actually
a problem? I fear so. The other way would be to put the local_error
handler as function pointers somewhere reachable from struct sock.
[PATCH RFC] xfrm: make local error reporting more robust
In xfrm4 and xfrm6 we need to take care about sockets of the other
address family. This could happen because a 6in4 or 4in6 tunnel could
get protected by ipsec.
If the skb->encapsulation bit is set, use the addresses of the inner ip
header instead of the outer one. This is currently working for IPv4 and
will be wired up for IPv6 in a future patch.
(intentionally removed signed-off)
Reported-by: <vi0oss@gmail.com>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
---
include/net/xfrm.h | 1 +
net/ipv4/xfrm4_output.c | 3 +--
net/ipv6/xfrm6_output.c | 18 +++++-------------
net/xfrm/xfrm_output.c | 26 ++++++++++++++++++++++++++
4 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 94ce082..11b73cb 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1476,6 +1476,7 @@ extern int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi,
extern int xfrm_input_resume(struct sk_buff *skb, int nexthdr);
extern int xfrm_output_resume(struct sk_buff *skb, int err);
extern int xfrm_output(struct sk_buff *skb);
+extern void xfrm_local_error(struct sk_buff *skb, unsigned int mtu);
extern int xfrm_inner_extract_output(struct xfrm_state *x, struct sk_buff *skb);
extern int xfrm4_extract_header(struct sk_buff *skb);
extern int xfrm4_extract_input(struct xfrm_state *x, struct sk_buff *skb);
diff --git a/net/ipv4/xfrm4_output.c b/net/ipv4/xfrm4_output.c
index 327a617..cfc386f 100644
--- a/net/ipv4/xfrm4_output.c
+++ b/net/ipv4/xfrm4_output.c
@@ -33,8 +33,7 @@ static int xfrm4_tunnel_check_size(struct sk_buff *skb)
mtu = dst_mtu(dst);
if (skb->len > mtu) {
if (skb->sk)
- ip_local_error(skb->sk, EMSGSIZE, ip_hdr(skb)->daddr,
- inet_sk(skb->sk)->inet_dport, mtu);
+ xfrm_local_error(skb, mtu);
else
icmp_send(skb, ICMP_DEST_UNREACH,
ICMP_FRAG_NEEDED, htonl(mtu));
diff --git a/net/ipv6/xfrm6_output.c b/net/ipv6/xfrm6_output.c
index 8755a30..7970965 100644
--- a/net/ipv6/xfrm6_output.c
+++ b/net/ipv6/xfrm6_output.c
@@ -48,23 +48,15 @@ static void xfrm6_local_rxpmtu(struct sk_buff *skb, u32 mtu)
struct flowi6 fl6;
struct sock *sk = skb->sk;
+ if (sk->sk_family != AF_INET6)
+ return;
+
fl6.flowi6_oif = sk->sk_bound_dev_if;
fl6.daddr = ipv6_hdr(skb)->daddr;
ipv6_local_rxpmtu(sk, &fl6, mtu);
}
-static void xfrm6_local_error(struct sk_buff *skb, u32 mtu)
-{
- struct flowi6 fl6;
- struct sock *sk = skb->sk;
-
- fl6.fl6_dport = inet_sk(sk)->inet_dport;
- fl6.daddr = ipv6_hdr(skb)->daddr;
-
- ipv6_local_error(sk, EMSGSIZE, &fl6, mtu);
-}
-
static int xfrm6_tunnel_check_size(struct sk_buff *skb)
{
int mtu, ret = 0;
@@ -80,7 +72,7 @@ static int xfrm6_tunnel_check_size(struct sk_buff *skb)
if (xfrm6_local_dontfrag(skb))
xfrm6_local_rxpmtu(skb, mtu);
else if (skb->sk)
- xfrm6_local_error(skb, mtu);
+ xfrm_local_error(skb, mtu);
else
icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
ret = -EMSGSIZE;
@@ -142,7 +134,7 @@ static int __xfrm6_output(struct sk_buff *skb)
xfrm6_local_rxpmtu(skb, mtu);
return -EMSGSIZE;
} else if (!skb->local_df && skb->len > mtu && skb->sk) {
- xfrm6_local_error(skb, mtu);
+ xfrm_local_error(skb, mtu);
return -EMSGSIZE;
}
diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index eb4a842..2a16cb8 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -214,5 +214,31 @@ int xfrm_inner_extract_output(struct xfrm_state *x, struct sk_buff *skb)
return inner_mode->afinfo->extract_output(x, skb);
}
+void xfrm_local_error(struct sk_buff *skb, unsigned int mtu)
+{
+ switch (skb->sk->sk_family) {
+ case AF_INET: {
+ struct iphdr *hdr = skb->encapsulation ? inner_ip_hdr(skb) :
+ ip_hdr(skb);
+ ip_local_error(skb->sk, EMSGSIZE, hdr->daddr,
+ inet_sk(skb->sk)->inet_dport, mtu);
+ return;
+ }
+#if IS_ENABLED(CONFIG_IPV6)
+ case AF_INET6: {
+ struct flowi6 fl6;
+ struct ipv6hdr *hdr = skb->encapsulation ? inner_ipv6_hdr(skb) :
+ inner_ipv6_hdr(skb);
+ fl6.fl6_dport = inet_sk(skb->sk)->inet_dport;
+ fl6.daddr = hdr->daddr;
+ ipv6_local_error(skb->sk, EMSGSIZE, &fl6, mtu);
+ return;
+ }
+#endif
+ }
+ BUG();
+}
+
EXPORT_SYMBOL_GPL(xfrm_output);
EXPORT_SYMBOL_GPL(xfrm_inner_extract_output);
+EXPORT_SYMBOL_GPL(xfrm_local_error);
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 3/4] drivers/net: enic: Make ASIC information available to USNIC
From: Ben Hutchings @ 2013-08-10 16:13 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Neel Patel, netdev, Nishank Trivedi, Christian Benvenuti
In-Reply-To: <20130809152135.11c19869@nehalam.linuxnetplumber.net>
On Fri, 2013-08-09 at 15:21 -0700, Stephen Hemminger wrote:
> On Fri, 9 Aug 2013 11:12:20 -0700
> Neel Patel <neepatel@cisco.com> wrote:
>
> > This patch provides asic information via ethtool.
[...]
> > --- a/drivers/net/ethernet/cisco/enic/enic_ethtool.c
> > +++ b/drivers/net/ethernet/cisco/enic/enic_ethtool.c
> > @@ -19,6 +19,7 @@
> > #include <linux/netdevice.h>
> > #include <linux/ethtool.h>
> >
> > +#include "driver_utils.h"
> > #include "enic_res.h"
> > #include "enic.h"
> > #include "enic_dev.h"
> > @@ -116,6 +117,9 @@ static void enic_get_drvinfo(struct net_device *netdev,
> > sizeof(drvinfo->fw_version));
> > strlcpy(drvinfo->bus_info, pci_name(enic->pdev),
> > sizeof(drvinfo->bus_info));
> > + memset(drvinfo->reserved1, 0, sizeof(drvinfo->reserved1));
> > + driver_encode_asic_info(drvinfo->reserved1, sizeof(drvinfo->reserved1),
> > + fw_info->asic_type, fw_info->asic_rev);
> > }
>
> If you want to use a reserved field, then make it a first class citizen.
> Rename it to asic_info, make sure the result is okay for other drivers
> and add send patch so Ben can make it part of normal ethtool support.
>
> Otherwise, this code is likely to break when someone else actually unreserves
> that field.
Right. I bet this is redundant with the IDs that lspci can show,
anyway.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH] netfilter: nf_conntrack: fix tcp_in_window for Fast Open
From: Jozsef Kadlecsik @ 2013-08-10 13:01 UTC (permalink / raw)
To: Yuchung Cheng; +Cc: pablo, netfilter-devel, netdev, edumazet
In-Reply-To: <1376094087-17700-1-git-send-email-ycheng@google.com>
On Fri, 9 Aug 2013, Yuchung Cheng wrote:
> Currently the conntrack checks if the ending sequence of a packet
> falls within the observed receive window. However it does so even
> if it has not observe any packet from the remote yet and uses an
> uninitialized receive window (td_maxwin).
>
> If a connection uses Fast Open to send a SYN-data packet which is
> dropped afterward in the network. The subsequent SYNs retransmits
> will all fail this check and be discarded, leading to a connection
> timeout. This is because the SYN retransmit does not contain data
> payload so
>
> end == initial sequence number (isn) + 1
> sender->td_end == isn + syn_data_len
> receiver->td_maxwin == 0
>
> The fix is to only apply this check after td_maxwin is initialized.
>
> Reported-by: Michael Chan <mcfchan@stanford.edu>
> Signed-off-by: Yuchung Cheng <ycheng@google.com>
Acked-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
> ---
> net/netfilter/nf_conntrack_proto_tcp.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
> index 4d4d8f1..e0f9a32 100644
> --- a/net/netfilter/nf_conntrack_proto_tcp.c
> +++ b/net/netfilter/nf_conntrack_proto_tcp.c
> @@ -526,7 +526,7 @@ static bool tcp_in_window(const struct nf_conn *ct,
> const struct nf_conntrack_tuple *tuple = &ct->tuplehash[dir].tuple;
> __u32 seq, ack, sack, end, win, swin;
> s16 receiver_offset;
> - bool res;
> + bool res, in_recv_win;
>
> /*
> * Get the required data from the packet.
> @@ -649,14 +649,18 @@ static bool tcp_in_window(const struct nf_conn *ct,
> receiver->td_end, receiver->td_maxend, receiver->td_maxwin,
> receiver->td_scale);
>
> + /* Is the ending sequence in the receive window (if available)? */
> + in_recv_win = !receiver->td_maxwin ||
> + after(end, sender->td_end - receiver->td_maxwin - 1);
> +
> pr_debug("tcp_in_window: I=%i II=%i III=%i IV=%i\n",
> before(seq, sender->td_maxend + 1),
> - after(end, sender->td_end - receiver->td_maxwin - 1),
> + (in_recv_win ? 1 : 0),
> before(sack, receiver->td_end + 1),
> after(sack, receiver->td_end - MAXACKWINDOW(sender) - 1));
>
> if (before(seq, sender->td_maxend + 1) &&
> - after(end, sender->td_end - receiver->td_maxwin - 1) &&
> + in_recv_win &&
> before(sack, receiver->td_end + 1) &&
> after(sack, receiver->td_end - MAXACKWINDOW(sender) - 1)) {
> /*
> @@ -725,7 +729,7 @@ static bool tcp_in_window(const struct nf_conn *ct,
> nf_log_packet(net, pf, 0, skb, NULL, NULL, NULL,
> "nf_ct_tcp: %s ",
> before(seq, sender->td_maxend + 1) ?
> - after(end, sender->td_end - receiver->td_maxwin - 1) ?
> + in_recv_win ?
> before(sack, receiver->td_end + 1) ?
> after(sack, receiver->td_end - MAXACKWINDOW(sender) - 1) ? "BUG"
> : "ACK is under the lower bound (possible overly delayed ACK)"
> --
> 1.8.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
-
E-mail : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
H-1525 Budapest 114, POB. 49, Hungary
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox