Re: [PATCH for-next] mlx4_core: Add support for secure-host and SMP firewall
On 01/06/2014 11:53, Or Gerlitz wrote: Secure-host is the general term for the capability of a device to protect itself and the subnet from malicious host software. Roland, are you planning to pull this into your next update, it's just -rc1 so I guess this is early enough to get that into 3.16 Or. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/1] IPoIB: Improve parent interface p_key handling.
On 11/06/2014 22:22, Alex Estrin wrote:
With reference to commit c2904141696ee19551f155396f23cdd5d95e
(IPoIB: Fix pkey change flow for virtualization environments)
It was noticed that parent interface keeps sending broadcast group
join requests if p_key index 0 is invalid. That creates unnecessary
noise on a fabric:
ib0: multicast join failed for ff12:401b:8000:::::,
status -22
Proposed patch re-init resources, then brings interface
up only if p_key idx 0 is valid either on bootup or on PKEY_CHANGE event.
Original code could run multicast task regardless of p_key value,
which we want to avoid.
Reviewed-by: Ira Weiny
Signed-off-by: Alex Estrin
Hi Alex, I re-wrote the change-log to make it clearer and also follow some
practices we use in the kernel (mentioned them to you over prev posts,
but maybe
it wasn't clear enough...) also see some comments further down the patch --
IPoIB: Avoid multicast join attempts when having invalid p_key
Currently, the parent interface keeps sending broadcast group join
requests even if p_key index 0 is invalid, which for itself is
possible/common in virtualized environmentswhere a VF has been probed to
VM but the actual p_key configuration has not yet been assigned by the
management software. This creates unnecessary noise on the fabric and in
the kernel logs:
ib0: multicast join failed for ff12:401b:8000:::::,
status -22
The original code run the multicast task regardless of the actual
p_key value, which can be avoided. The fix is to re-init resources and
bring interface up only if p_key index 0 is valid either when starting
up or on PKEY_CHANGE event.
Fixes: c290414169 ('IPoIB: Fix pkey change flow for virtualization
environments')
---
drivers/infiniband/ulp/ipoib/ipoib_ib.c | 35 +++
1 files changed, 21 insertions(+), 14 deletions(-)
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ib.c
b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
index 6a7003d..a2af996 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
@@ -52,6 +52,7 @@ MODULE_PARM_DESC(data_debug_level,
#endif
static DEFINE_MUTEX(pkey_mutex);
+static void ipoib_pkey_dev_check_presence(struct net_device *dev);
struct ipoib_ah *ipoib_create_ah(struct net_device *dev,
struct ib_pd *pd, struct ib_ah_attr *attr)
@@ -669,12 +670,13 @@ int ipoib_ib_dev_open(struct net_device *dev)
struct ipoib_dev_priv *priv = netdev_priv(dev);
int ret;
- if (ib_find_pkey(priv->ca, priv->port, priv->pkey, &priv->pkey_index)) {
- ipoib_warn(priv, "P_Key 0x%04x not found\n", priv->pkey);
- clear_bit(IPOIB_PKEY_ASSIGNED, &priv->flags);
+ ipoib_pkey_dev_check_presence(dev);
+
+ if (!test_bit(IPOIB_PKEY_ASSIGNED, &priv->flags)) {
+ ipoib_warn(priv, "P_Key 0x%04x is %s\n", priv->pkey,
+ !(priv->pkey & 0x7fff) ? "Invalid" : "not found");
CHECK: Alignment should match open parenthesis
#44: FILE: drivers/infiniband/ulp/ipoib/ipoib_ib.c:677:
+ ipoib_warn(priv, "P_Key 0x%04x is %s\n", priv->pkey,
+ !(priv->pkey & 0x7fff) ? "Invalid" : "not found");
please run checkpatch --strict on your patch to fix this one and it's
such (there are more) basically the guideline (based on a quote ofDave
Miller ...) is:
when you have a multi-line function call or if () conditional,the first
non-whitespace character on the second and subsequent lines should line
up with the first column after the openning parenthesis on the first line.
@@ -1038,8 +1043,10 @@ static void __ipoib_ib_dev_flush(struct ipoib_dev_priv
*priv,
ipoib_ib_dev_down(dev, 0);
if (level == IPOIB_FLUSH_HEAVY) {
- ipoib_ib_dev_stop(dev, 0);
- ipoib_ib_dev_open(dev);
+ if (test_bit(IPOIB_FLAG_INITIALIZED, &priv->flags))
+ ipoib_ib_dev_stop(dev, 0);
+ if (ipoib_ib_dev_open(dev) != 0)
+ return;
}
is testing the return code of ipoib_ib_dev_open() really part of the
functional/fix change introduced by this patch or a different fix? if
the latter, put to different/future patch
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] infiniband: Fixes memory leak in send_flowc
On Mon, 16 Jun 2014, Nick Krause wrote:
> That's true David,
> I will resend this parch without the use of the pr_warn.
There's no patch to resend if you don't use pr_warn(). kfree_skb(skb) is
unnecessary if !skb, look at the first thing it checks:
void kfree_skb(struct sk_buff *skb)
{
if (unlikely(!skb))
return;
...
}
Thus, I don't see the memory leak you're referring to.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] infiniband: Fixes memory leak in send_flowc
On Mon, 16 Jun 2014, Nick Krause wrote: > If that is the case ,David I would mark bug id 44631 as closed due to no > need for my if statement. You don't want to depend on the implementation of the page allocator to never return NULL for orders < PAGE_ALLOC_COSTLY_ORDER with GFP_KERNEL, it could possibly change in the future and we wouldn't catch your dependency in send_flowc(). The size object size of the skbuff_head_cache slab cache could also change. You don't need the suggested pr_warn(), though, since the page allocation failure warning would also be noisy enough. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] infiniband: Fixes memory leak in send_flowc
On Mon, 16 Jun 2014, Steve Wise wrote:
> On 6/16/2014 12:49 PM, Nicholas Krause wrote:
> > Signed-off-by: Nicholas Krause
> > ---
> > drivers/infiniband/hw/cxgb4/cm.c | 5 -
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/infiniband/hw/cxgb4/cm.c
> > b/drivers/infiniband/hw/cxgb4/cm.c
> > index 5e153f6..867e664 100644
> > --- a/drivers/infiniband/hw/cxgb4/cm.c
> > +++ b/drivers/infiniband/hw/cxgb4/cm.c
> > @@ -455,8 +455,11 @@ static void send_flowc(struct c4iw_ep *ep, struct
> > sk_buff *skb)
> > unsigned int flowclen = 80;
> > struct fw_flowc_wr *flowc;
> > int i;
> > -
>
> Please add back the above blank line.
>
> > skb = get_skb(skb, flowclen, GFP_KERNEL);
> > + if (!skb) {
> > + kfree_skb(skb);
>
> Let's do a pr_warn() here;
>
> pr_warn(MOD "%s failed to allocate skb.\n", __func__);
>
No need, if the allocation from skbuff_head_cache fails in the slab
allocator, the page allocator will loop forever for orders less than
PAGE_ALLOC_COSTLY_ORDER or spew a page allocation failure warning with a
stack trace that indicated the high order page allocation failed in this
path.
>
> > + return;
> > + }
> > flowc = (struct fw_flowc_wr *)__skb_put(skb, flowclen);
> > flowc->op_to_nparams = cpu_to_be32(FW_WR_OP(FW_FLOWC_WR) |
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2] iw_cxgb4: in send_flowc(), handle a skb return from get_skb()
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index 5e153f6..e0c43a3 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -457,6 +457,10 @@ static void send_flowc(struct c4iw_ep *ep, struct sk_buff
*skb)
int i;
skb = get_skb(skb, flowclen, GFP_KERNEL);
+ if (!skb) {
+ pr_warn(MOD "%s failed to allocate skb.\n", __func__);
+ return;
+ }
flowc = (struct fw_flowc_wr *)__skb_put(skb, flowclen);
flowc->op_to_nparams = cpu_to_be32(FW_WR_OP(FW_FLOWC_WR) |
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2] iw_cxgb4: in send_flowc(), handle a skb return from get_skb()
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index 5e153f6..e0c43a3 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -457,6 +457,10 @@ static void send_flowc(struct c4iw_ep *ep, struct sk_buff
*skb)
int i;
skb = get_skb(skb, flowclen, GFP_KERNEL);
+ if (!skb) {
+ pr_warn(MOD "%s failed to allocate skb.\n", __func__);
+ return;
+ }
flowc = (struct fw_flowc_wr *)__skb_put(skb, flowclen);
flowc->op_to_nparams = cpu_to_be32(FW_WR_OP(FW_FLOWC_WR) |
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2] iw_cxgb4: in send_flowc(), handle a skb return from get_skb()
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index 5e153f6..e0c43a3 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -457,6 +457,10 @@ static void send_flowc(struct c4iw_ep *ep, struct sk_buff
*skb)
int i;
skb = get_skb(skb, flowclen, GFP_KERNEL);
+ if (!skb) {
+ pr_warn(MOD "%s failed to allocate skb.\n", __func__);
+ return;
+ }
flowc = (struct fw_flowc_wr *)__skb_put(skb, flowclen);
flowc->op_to_nparams = cpu_to_be32(FW_WR_OP(FW_FLOWC_WR) |
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2] iw_cxgb4: in send_flowc(), handle a skb return from get_skb()
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index 5e153f6..e0c43a3 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -457,6 +457,10 @@ static void send_flowc(struct c4iw_ep *ep, struct sk_buff
*skb)
int i;
skb = get_skb(skb, flowclen, GFP_KERNEL);
+ if (!skb) {
+ kfree_skb();
+ pr_warn(MOD "%s failed to allocate skb.\n", __func__);
+ }
flowc = (struct fw_flowc_wr *)__skb_put(skb, flowclen);
flowc->op_to_nparams = cpu_to_be32(FW_WR_OP(FW_FLOWC_WR) |
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] infinibad: fix memory leak in function send_flowc
On 6/16/2014 1:34 PM, Steve Wise wrote:
On 6/16/2014 1:30 PM, Nicholas Krause wrote:
Signed-off-by: Nicholas Krause
---
drivers/infiniband/hw/cxgb4/cm.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/cxgb4/cm.c
b/drivers/infiniband/hw/cxgb4/cm.c
index 5e153f6..c518411 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -455,7 +455,10 @@ static void send_flowc(struct c4iw_ep *ep,
struct sk_buff *skb)
unsigned int flowclen = 80;
struct fw_flowc_wr *flowc;
int i;
+if (!skb) {
+kfree_skb();
+pr_warn(MOD "%s failed to allocate skb.\n", __func__);
+}
skb = get_skb(skb, flowclen, GFP_KERNEL);
Oops, shouldn't the if statement be after the call to get_skb()? :)
(you're having a bad day ;))
And you don't free anything. If get_skb() returns NULL, then log the
warning and return.
flowc = (struct fw_flowc_wr *)__skb_put(skb, flowclen);
One more nit: The comment is no longer correct. It should be
something like:
iw_cxgb4: in send_flowc(), handle a null skb return from get_skb()
--
To unsubscribe from this list: send the line "unsubscribe
linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] infinibad: fix memory leak in function send_flowc
On 6/16/2014 1:30 PM, Nicholas Krause wrote:
Signed-off-by: Nicholas Krause
---
drivers/infiniband/hw/cxgb4/cm.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index 5e153f6..c518411 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -455,7 +455,10 @@ static void send_flowc(struct c4iw_ep *ep, struct sk_buff
*skb)
unsigned int flowclen = 80;
struct fw_flowc_wr *flowc;
int i;
+ if (!skb) {
+ kfree_skb();
+ pr_warn(MOD "%s failed to allocate skb.\n", __func__);
+ }
skb = get_skb(skb, flowclen, GFP_KERNEL);
Oops, shouldn't the if statement be after the call to get_skb()? :)
(you're having a bad day ;))
flowc = (struct fw_flowc_wr *)__skb_put(skb, flowclen);
One more nit: The comment is no longer correct. It should be something
like:
iw_cxgb4: in send_flowc(), handle a null skb return from get_skb()
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] infinibad: fix memory leak in function send_flowc
Signed-off-by: Nicholas Krause
---
drivers/infiniband/hw/cxgb4/cm.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index 5e153f6..c518411 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -455,7 +455,10 @@ static void send_flowc(struct c4iw_ep *ep, struct sk_buff
*skb)
unsigned int flowclen = 80;
struct fw_flowc_wr *flowc;
int i;
+ if (!skb) {
+ kfree_skb();
+ pr_warn(MOD "%s failed to allocate skb.\n", __func__);
+ }
skb = get_skb(skb, flowclen, GFP_KERNEL);
flowc = (struct fw_flowc_wr *)__skb_put(skb, flowclen);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] infiniband: Fixes memory leak in send_flowc
On 6/16/2014 12:49 PM, Nicholas Krause wrote:
Signed-off-by: Nicholas Krause
---
drivers/infiniband/hw/cxgb4/cm.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index 5e153f6..867e664 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -455,8 +455,11 @@ static void send_flowc(struct c4iw_ep *ep, struct sk_buff
*skb)
unsigned int flowclen = 80;
struct fw_flowc_wr *flowc;
int i;
-
Please add back the above blank line.
skb = get_skb(skb, flowclen, GFP_KERNEL);
+ if (!skb) {
+ kfree_skb(skb);
Let's do a pr_warn() here;
pr_warn(MOD "%s failed to allocate skb.\n", __func__);
+ return;
+ }
flowc = (struct fw_flowc_wr *)__skb_put(skb, flowclen);
flowc->op_to_nparams = cpu_to_be32(FW_WR_OP(FW_FLOWC_WR) |
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] infiniband: Fixes memory leak in send_flowc
Signed-off-by: Nicholas Krause
---
drivers/infiniband/hw/cxgb4/cm.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index 5e153f6..867e664 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -455,8 +455,11 @@ static void send_flowc(struct c4iw_ep *ep, struct sk_buff
*skb)
unsigned int flowclen = 80;
struct fw_flowc_wr *flowc;
int i;
-
skb = get_skb(skb, flowclen, GFP_KERNEL);
+ if (!skb) {
+ kfree_skb(skb);
+ return;
+ }
flowc = (struct fw_flowc_wr *)__skb_put(skb, flowclen);
flowc->op_to_nparams = cpu_to_be32(FW_WR_OP(FW_FLOWC_WR) |
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv4]Checks for Null value in function *get_skub
On 6/16/2014 10:45 AM, Nick Krause wrote: Can we close this bug or is it still a issue as we can' return NULL from kfree or kfree_skb. Here is the bug ID 44631 I would close it if we are done with the bug otherwise I think there are no other issues with allocating a skb, In what database is this bug ID 44631? -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch Resend] infiniband:memory leak in get_skb function in file cm.c
On 6/16/2014 12:06 PM, Nicholas Krause wrote:
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index f9477e2..2d56983 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -340,15 +340,13 @@ static int status2errno(int status)
*/
static struct sk_buff *get_skb(struct sk_buff *skb, int len, gfp_t gfp)
{
if (skb && !skb_is_nonlinear(skb) && !skb_cloned(skb)) {
skb_trim(skb, 0);
skb_get(skb);
skb_reset_transport_header(skb);
} else {
+if (!skb)
+ skb_kfree(skb);
+ /*NULL is ignored */
skb = alloc_skb(len, gfp);
This is still incorrect and the formatting is screwy.
As we discussed, you can call skb_kfree() with a NULL skb and it will
silently ignore it. So you don't need th 'if (!skb)'...
Perhaps you should read up more on submitting patches to get the
formatting correct, and how to use the various tools, etc.
steve.
}
t4_set_arp_err_handler(skb, NULL, NULL);
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[Patch Resend] infiniband:memory leak in get_skb function in file cm.c
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index f9477e2..2d56983 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -340,15 +340,13 @@ static int status2errno(int status)
*/
static struct sk_buff *get_skb(struct sk_buff *skb, int len, gfp_t gfp)
{
if (skb && !skb_is_nonlinear(skb) && !skb_cloned(skb)) {
skb_trim(skb, 0);
skb_get(skb);
skb_reset_transport_header(skb);
} else {
+if (!skb)
+ skb_kfree(skb);
+ /*NULL is ignored */
skb = alloc_skb(len, gfp);
}
t4_set_arp_err_handler(skb, NULL, NULL);
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv4]Checks for Null value in function *get_skub
On 6/16/2014 10:45 AM, Levente Kurusa wrote:
On 06/16/2014 05:37 PM, Nick Krause wrote:
Signed-off-by: Nicholas Krause
---
drivers/infiniband/hw/cxgb4/
cm.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/infiniband/hw/cxgb4/cm.c
b/drivers/infiniband/hw/cxgb4/cm.c
index f9477e2..2d56983 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -340,15 +340,13 @@ static int status2errno(int status)
*/
static struct sk_buff *get_skb(struct sk_buff *skb, int len, gfp_t
gfp)
{
if (skb && !skb_is_nonlinear(skb) && !skb_cloned(skb)) {
skb_trim(skb, 0);
skb_get(skb);
skb_reset_transport_header(skb);
} else {
+ if (skb)
/* NULL is ignored */.
+ kfree_skb (skb);
skb = alloc_skb(len, gfp);
}
t4_set_arp_err_handler(skb, NULL, NULL);
This patch is severely whitespace damaged. Can you fix your MUA
or try to use git-send-email?
The changelog as well is of low quality. Maybe something like this:
infiniband: cxgb4: fix memory skb memory leak
... would be better.
Also, this fails the build, due to the extra dot character after
the comment. :-(
Agreed. Start over with this patch, do what we asked, and make sure it
passes checkpatch...
Thanks,
Levente Kurusa.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv4]Checks for Null value in function *get_skub
Hey Guys ,
It states tot run cleanfile or cleanpatch script but it doesn't work
how do I run these scripts.
Thanks Nick
On Mon, Jun 16, 2014 at 11:45 AM, Levente Kurusa wrote:
> On 06/16/2014 05:37 PM, Nick Krause wrote:
>>
>> Signed-off-by: Nicholas Krause
>> ---
>> drivers/infiniband/hw/cxgb4/
>> cm.c | 8 +++-
>> 1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/infiniband/hw/cxgb4/cm.c
>> b/drivers/infiniband/hw/cxgb4/cm.c
>> index f9477e2..2d56983 100644
>> --- a/drivers/infiniband/hw/cxgb4/cm.c
>> +++ b/drivers/infiniband/hw/cxgb4/cm.c
>> @@ -340,15 +340,13 @@ static int status2errno(int status)
>>*/
>> static struct sk_buff *get_skb(struct sk_buff *skb, int len, gfp_t gfp)
>> {
>> if (skb && !skb_is_nonlinear(skb) && !skb_cloned(skb)) {
>> skb_trim(skb, 0);
>> skb_get(skb);
>> skb_reset_transport_header(skb);
>> } else {
>> + if (skb)
>> /* NULL is ignored */.
>> + kfree_skb (skb);
>> skb = alloc_skb(len, gfp);
>> }
>> t4_set_arp_err_handler(skb, NULL, NULL);
>
>
> This patch is severely whitespace damaged. Can you fix your MUA
> or try to use git-send-email?
>
> The changelog as well is of low quality. Maybe something like this:
>
> infiniband: cxgb4: fix memory skb memory leak
>
> ... would be better.
>
> Also, this fails the build, due to the extra dot character after
> the comment. :-(
>
> Thanks,
> Levente Kurusa.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv4]Checks for Null value in function *get_skub
On 06/16/2014 05:37 PM, Nick Krause wrote:
Signed-off-by: Nicholas Krause
---
drivers/infiniband/hw/cxgb4/
cm.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index f9477e2..2d56983 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -340,15 +340,13 @@ static int status2errno(int status)
*/
static struct sk_buff *get_skb(struct sk_buff *skb, int len, gfp_t gfp)
{
if (skb && !skb_is_nonlinear(skb) && !skb_cloned(skb)) {
skb_trim(skb, 0);
skb_get(skb);
skb_reset_transport_header(skb);
} else {
+ if (skb)
/* NULL is ignored */.
+ kfree_skb (skb);
skb = alloc_skb(len, gfp);
}
t4_set_arp_err_handler(skb, NULL, NULL);
This patch is severely whitespace damaged. Can you fix your MUA
or try to use git-send-email?
The changelog as well is of low quality. Maybe something like this:
infiniband: cxgb4: fix memory skb memory leak
... would be better.
Also, this fails the build, due to the extra dot character after
the comment. :-(
Thanks,
Levente Kurusa.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv4]Checks for Null value in function *get_skub
Can we close this bug or is it still a issue as we can'
return NULL from kfree or kfree_skb. Here is the bug
ID 44631 I would close it if we are done with the bug
otherwise I think there are no other issues with
allocating a skb,
Cheers Nick
On Mon, Jun 16, 2014 at 11:37 AM, Nick Krause wrote:
> Signed-off-by: Nicholas Krause
> ---
> drivers/infiniband/hw/cxgb4/
> cm.c | 8 +++-
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/infiniband/hw/cxgb4/cm.c
> b/drivers/infiniband/hw/cxgb4/cm.c
> index f9477e2..2d56983 100644
> --- a/drivers/infiniband/hw/cxgb4/cm.c
> +++ b/drivers/infiniband/hw/cxgb4/cm.c
> @@ -340,15 +340,13 @@ static int status2errno(int status)
> */
> static struct sk_buff *get_skb(struct sk_buff *skb, int len, gfp_t gfp)
> {
> if (skb && !skb_is_nonlinear(skb) && !skb_cloned(skb)) {
> skb_trim(skb, 0);
> skb_get(skb);
> skb_reset_transport_header(skb);
> } else {
> + if (skb)
> /* NULL is ignored */.
> + kfree_skb (skb);
> skb = alloc_skb(len, gfp);
> }
> t4_set_arp_err_handler(skb, NULL, NULL);
>
> On Mon, Jun 16, 2014 at 11:30 AM, Steve Wise
> wrote:
>> On 6/16/2014 10:25 AM, Nicholas Krause wrote:
>>>
>>> Signed-off-by: Nicholas Krause
>>> ---
>>> drivers/infiniband/hw/cxgb4/cm.c | 8 +++-
>>> 1 file changed, 3 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/infiniband/hw/cxgb4/cm.c
>>> b/drivers/infiniband/hw/cxgb4/cm.c
>>> index f9477e2..2d56983 100644
>>> --- a/drivers/infiniband/hw/cxgb4/cm.c
>>> +++ b/drivers/infiniband/hw/cxgb4/cm.c
>>> @@ -340,15 +340,13 @@ static int status2errno(int status)
>>>*/
>>> static struct sk_buff *get_skb(struct sk_buff *skb, int len, gfp_t gfp)
>>> {
>>> if (skb && !skb_is_nonlinear(skb) && !skb_cloned(skb)) {
>>> skb_trim(skb, 0);
>>> skb_get(skb);
>>> skb_reset_transport_header(skb);
>>> } else {
>>> + if (skb)
>>> + kfree_skb (skb);
>>> skb = alloc_skb(len, gfp);
>>> }
>>> t4_set_arp_err_handler(skb, NULL, NULL);
>>
>>
>> Can you change the comment? This patch is now fixing a potential skb leak.
>> Also, kfree_sb() will ignore NULL ptrs, so we could just always call it.
>> But I'd add a comment like /* NULL is ignored */.
>>
>> Thanks,
>>
>> Steve.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv4]Checks for Null value in function *get_skub
Signed-off-by: Nicholas Krause
---
drivers/infiniband/hw/cxgb4/
cm.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index f9477e2..2d56983 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -340,15 +340,13 @@ static int status2errno(int status)
*/
static struct sk_buff *get_skb(struct sk_buff *skb, int len, gfp_t gfp)
{
if (skb && !skb_is_nonlinear(skb) && !skb_cloned(skb)) {
skb_trim(skb, 0);
skb_get(skb);
skb_reset_transport_header(skb);
} else {
+ if (skb)
/* NULL is ignored */.
+ kfree_skb (skb);
skb = alloc_skb(len, gfp);
}
t4_set_arp_err_handler(skb, NULL, NULL);
On Mon, Jun 16, 2014 at 11:30 AM, Steve Wise
wrote:
> On 6/16/2014 10:25 AM, Nicholas Krause wrote:
>>
>> Signed-off-by: Nicholas Krause
>> ---
>> drivers/infiniband/hw/cxgb4/cm.c | 8 +++-
>> 1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/infiniband/hw/cxgb4/cm.c
>> b/drivers/infiniband/hw/cxgb4/cm.c
>> index f9477e2..2d56983 100644
>> --- a/drivers/infiniband/hw/cxgb4/cm.c
>> +++ b/drivers/infiniband/hw/cxgb4/cm.c
>> @@ -340,15 +340,13 @@ static int status2errno(int status)
>>*/
>> static struct sk_buff *get_skb(struct sk_buff *skb, int len, gfp_t gfp)
>> {
>> if (skb && !skb_is_nonlinear(skb) && !skb_cloned(skb)) {
>> skb_trim(skb, 0);
>> skb_get(skb);
>> skb_reset_transport_header(skb);
>> } else {
>> + if (skb)
>> + kfree_skb (skb);
>> skb = alloc_skb(len, gfp);
>> }
>> t4_set_arp_err_handler(skb, NULL, NULL);
>
>
> Can you change the comment? This patch is now fixing a potential skb leak.
> Also, kfree_sb() will ignore NULL ptrs, so we could just always call it.
> But I'd add a comment like /* NULL is ignored */.
>
> Thanks,
>
> Steve.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv4]Checks for Null value in function *get_skub
On 06/16/2014 05:30 PM, Steve Wise wrote:
On 6/16/2014 10:25 AM, Nicholas Krause wrote:
Signed-off-by: Nicholas Krause
---
drivers/infiniband/hw/cxgb4/cm.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index f9477e2..2d56983 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -340,15 +340,13 @@ static int status2errno(int status)
*/
static struct sk_buff *get_skb(struct sk_buff *skb, int len, gfp_t gfp)
{
if (skb && !skb_is_nonlinear(skb) && !skb_cloned(skb)) {
skb_trim(skb, 0);
skb_get(skb);
skb_reset_transport_header(skb);
} else {
+if (skb)
+kfree_skb (skb);
skb = alloc_skb(len, gfp);
}
t4_set_arp_err_handler(skb, NULL, NULL);
Can you change the comment? This patch is now fixing a potential skb leak.
Also, kfree_sb() will ignore NULL ptrs, so we could just always call it.
> But I'd add a comment like /* NULL is ignored */.
AFAIK, checkpatch.pl will show you a message if it detects this:
if (x)
kfree(x);
so I guess it is not really needed.
Thanks,
Levente Kurusa.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv4]Checks for Null value in function *get_skub
On 6/16/2014 10:25 AM, Nicholas Krause wrote:
Signed-off-by: Nicholas Krause
---
drivers/infiniband/hw/cxgb4/cm.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index f9477e2..2d56983 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -340,15 +340,13 @@ static int status2errno(int status)
*/
static struct sk_buff *get_skb(struct sk_buff *skb, int len, gfp_t gfp)
{
if (skb && !skb_is_nonlinear(skb) && !skb_cloned(skb)) {
skb_trim(skb, 0);
skb_get(skb);
skb_reset_transport_header(skb);
} else {
+ if (skb)
+ kfree_skb (skb);
skb = alloc_skb(len, gfp);
}
t4_set_arp_err_handler(skb, NULL, NULL);
Can you change the comment? This patch is now fixing a potential skb
leak. Also, kfree_sb() will ignore NULL ptrs, so we could just always
call it. But I'd add a comment like /* NULL is ignored */.
Thanks,
Steve.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv4]Checks for Null value in function *get_skub
Signed-off-by: Nicholas Krause
---
drivers/infiniband/hw/cxgb4/cm.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index f9477e2..2d56983 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -340,15 +340,13 @@ static int status2errno(int status)
*/
static struct sk_buff *get_skb(struct sk_buff *skb, int len, gfp_t gfp)
{
if (skb && !skb_is_nonlinear(skb) && !skb_cloned(skb)) {
skb_trim(skb, 0);
skb_get(skb);
skb_reset_transport_header(skb);
} else {
+ if (skb)
+ kfree_skb (skb);
skb = alloc_skb(len, gfp);
}
t4_set_arp_err_handler(skb, NULL, NULL);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3]Checks for Null value in function *get_skub
On 6/16/2014 10:14 AM, Nick Krause wrote:
>From what I know yes so I would close the bug at the the link in
my previous email and just void my patch.
Cheers Nick
We shouldn't be kfree-ing an skb anyway. Should use kfree_skb().
There is still a leak if skb is non-null and the skb is non linear or
cloned.
On Mon, Jun 16, 2014 at 11:12 AM, Levente Kurusa wrote:
On 06/16/2014 05:08 PM, Nicholas Krause wrote:
Signed-off-by: Nicholas Krause
---
drivers/infiniband/hw/cxgb4/cm.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/infiniband/hw/cxgb4/cm.c
b/drivers/infiniband/hw/cxgb4/cm.c
index f9477e2..2d56983 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -340,15 +340,13 @@ static int status2errno(int status)
*/
static struct sk_buff *get_skb(struct sk_buff *skb, int len, gfp_t gfp)
{
if (skb && !skb_is_nonlinear(skb) && !skb_cloned(skb)) {
skb_trim(skb, 0);
skb_get(skb);
skb_reset_transport_header(skb);
} else {
+ if (skb)
+ kfree (skb);
skb = alloc_skb(len, gfp);
}
t4_set_arp_err_handler(skb, NULL, NULL);
Isn't kfree(NULL) legal?
(i.e. the if statement is useless)
Thanks,
Levente Kurusa
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3]Checks for Null value in function *get_skub
>From what I know yes so I would close the bug at the the link in
my previous email and just void my patch.
Cheers Nick
On Mon, Jun 16, 2014 at 11:12 AM, Levente Kurusa wrote:
> On 06/16/2014 05:08 PM, Nicholas Krause wrote:
>>
>> Signed-off-by: Nicholas Krause
>> ---
>> drivers/infiniband/hw/cxgb4/cm.c | 8 +++-
>> 1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/infiniband/hw/cxgb4/cm.c
>> b/drivers/infiniband/hw/cxgb4/cm.c
>> index f9477e2..2d56983 100644
>> --- a/drivers/infiniband/hw/cxgb4/cm.c
>> +++ b/drivers/infiniband/hw/cxgb4/cm.c
>> @@ -340,15 +340,13 @@ static int status2errno(int status)
>>*/
>> static struct sk_buff *get_skb(struct sk_buff *skb, int len, gfp_t gfp)
>> {
>> if (skb && !skb_is_nonlinear(skb) && !skb_cloned(skb)) {
>> skb_trim(skb, 0);
>> skb_get(skb);
>> skb_reset_transport_header(skb);
>> } else {
>> + if (skb)
>> + kfree (skb);
>> skb = alloc_skb(len, gfp);
>> }
>> t4_set_arp_err_handler(skb, NULL, NULL);
>>
>
> Isn't kfree(NULL) legal?
>
> (i.e. the if statement is useless)
>
> Thanks,
> Levente Kurusa
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] Checks for Null value in function *get_skub
On 6/16/2014 10:03 AM, Nicholas Krause wrote:
Signed-off-by: Nicholas Krause
---
drivers/infiniband/hw/cxgb4/cm.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index f9477e2..2d56983 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -340,15 +340,13 @@ static int status2errno(int status)
*/
static struct sk_buff *get_skb(struct sk_buff *skb, int len, gfp_t gfp)
{
if (skb && !skb_is_nonlinear(skb) && !skb_cloned(skb)) {
skb_trim(skb, 0);
skb_get(skb);
skb_reset_transport_header(skb);
} else {
+ if (!skb)
+ kfree(skb);
skb = alloc_skb(len, gfp);
}
t4_set_arp_err_handler(skb, NULL, NULL);
Should be 'if (skb)'
:)
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3]Checks for Null value in function *get_skub
On 06/16/2014 05:08 PM, Nicholas Krause wrote:
Signed-off-by: Nicholas Krause
---
drivers/infiniband/hw/cxgb4/cm.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index f9477e2..2d56983 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -340,15 +340,13 @@ static int status2errno(int status)
*/
static struct sk_buff *get_skb(struct sk_buff *skb, int len, gfp_t gfp)
{
if (skb && !skb_is_nonlinear(skb) && !skb_cloned(skb)) {
skb_trim(skb, 0);
skb_get(skb);
skb_reset_transport_header(skb);
} else {
+ if (skb)
+ kfree (skb);
skb = alloc_skb(len, gfp);
}
t4_set_arp_err_handler(skb, NULL, NULL);
Isn't kfree(NULL) legal?
(i.e. the if statement is useless)
Thanks,
Levente Kurusa
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3]Checks for Null value in function *get_skub
The bug is open here https://bugzilla.kernel.org/show_bug.cgi?id=44631.
I would recommend closing it now as I seem to have fixed it.
Cheers Nick
On Mon, Jun 16, 2014 at 11:08 AM, Nicholas Krause wrote:
> Signed-off-by: Nicholas Krause
> ---
> drivers/infiniband/hw/cxgb4/cm.c | 8 +++-
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/infiniband/hw/cxgb4/cm.c
> b/drivers/infiniband/hw/cxgb4/cm.c
> index f9477e2..2d56983 100644
> --- a/drivers/infiniband/hw/cxgb4/cm.c
> +++ b/drivers/infiniband/hw/cxgb4/cm.c
> @@ -340,15 +340,13 @@ static int status2errno(int status)
> */
> static struct sk_buff *get_skb(struct sk_buff *skb, int len, gfp_t gfp)
> {
> if (skb && !skb_is_nonlinear(skb) && !skb_cloned(skb)) {
> skb_trim(skb, 0);
> skb_get(skb);
> skb_reset_transport_header(skb);
> } else {
> + if (skb)
> + kfree (skb);
> skb = alloc_skb(len, gfp);
> }
> t4_set_arp_err_handler(skb, NULL, NULL);
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3]Checks for Null value in function *get_skub
Signed-off-by: Nicholas Krause
---
drivers/infiniband/hw/cxgb4/cm.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index f9477e2..2d56983 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -340,15 +340,13 @@ static int status2errno(int status)
*/
static struct sk_buff *get_skb(struct sk_buff *skb, int len, gfp_t gfp)
{
if (skb && !skb_is_nonlinear(skb) && !skb_cloned(skb)) {
skb_trim(skb, 0);
skb_get(skb);
skb_reset_transport_header(skb);
} else {
+ if (skb)
+ kfree (skb);
skb = alloc_skb(len, gfp);
}
t4_set_arp_err_handler(skb, NULL, NULL);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2] Checks for Null value in function *get_skub
Signed-off-by: Nicholas Krause
---
drivers/infiniband/hw/cxgb4/cm.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index f9477e2..2d56983 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -340,15 +340,13 @@ static int status2errno(int status)
*/
static struct sk_buff *get_skb(struct sk_buff *skb, int len, gfp_t gfp)
{
if (skb && !skb_is_nonlinear(skb) && !skb_cloned(skb)) {
skb_trim(skb, 0);
skb_get(skb);
skb_reset_transport_header(skb);
} else {
+ if (!skb)
+ kfree(skb);
skb = alloc_skb(len, gfp);
}
t4_set_arp_err_handler(skb, NULL, NULL);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Fix memory leak in cm.c
On 6/15/2014 9:26 PM, Nick wrote:
This patch fixes memory by checking in function,
*get_skuff if it is be passing a Null skb
struct.
Cheers Nick
Signed-off-by: Nick
---
drivers/infiniband/hw/cxgb4/cm.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index 5e153f6..f9477e2 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -340,7 +340,11 @@ static int status2errno(int status)
*/
static struct sk_buff *get_skb(struct sk_buff *skb, int len, gfp_t gfp)
{
- if (skb && !skb_is_nonlinear(skb) && !skb_cloned(skb)) {
+ if (!skb) {
+ return NULL;
+ }
+
+ else if (skb && !skb_is_nonlinear(skb) && !skb_cloned(skb)) {
skb_trim(skb, 0);
skb_get(skb);
skb_reset_transport_header(skb);
This patch is incorrect. If skb is null, then get_skb() will allocate a
new one. The idea is to reuse skb if it can be reused (ie is linear and
not cloned). Otherwise allocate a new one:
static struct sk_buff *get_skb(struct sk_buff *skb, int len, gfp_t gfp)
{
if (skb && !skb_is_nonlinear(skb) && !skb_cloned(skb)) {
skb_trim(skb, 0);
skb_get(skb);
skb_reset_transport_header(skb);
} else {
skb = alloc_skb(len, gfp);
}
t4_set_arp_err_handler(skb, NULL, NULL);
return skb;
}
However, if skb is not null and (is not linear or is cloned), then we
leak the skb. So there is a bug.
So the else clause should free skb if it not NULL before calling
alloc_skb() to get a new one.
Steve.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH -next 08/26] infiniband: Use dma_zalloc_coherent
For cxgb3, Acked-by: Steve Wise -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
