Skip to contentSkip to conversation

New: [drivers/net/cxgb3/sge.c:563]: Possible memory leak: s

Bugme-new Andrew Morton 4 participants 5 messages Nov 23, 2008 – Nov 27, 2008 1 message not in the archive

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]/

Andrew Morton

author
#1 lore raw

(switched to email. Please respond via emailed reply-to-all, not via the bugzilla web interface).

Quoted 36 lines · On Sat, 22 Nov 2008 21:11:51 -0800 (PST) [email protected] wrote:

http://bugzilla.kernel.org/show_bug.cgi?id=12087

Summary: [drivers/net/cxgb3/sge.c:563]: Possible memory leak: s
Product: Drivers
Version: 2.5
KernelVersion: latest git tree
Platform: All
OS/Version: Linux
Tree: Mainline
Status: NEW
Severity: normal
Priority: P1
Component: Network
AssignedTo: [email protected]
ReportedBy: [email protected]

There will be a memory leak in the function alloc_ring when the parameter sw_size is nonzero and metadata is NULL. I don't know if that can happen.

Here is the code, taken from the latest git tree.

static void *alloc_ring(struct pci_dev *pdev, size_t nelem, size_t elem_size,
size_t sw_size, dma_addr_t * phys, void *metadata)

{
        size_t len = nelem * elem_size;
        void *s = NULL;
        void *p = dma_alloc_coherent(&pdev->dev, len, phys, GFP_KERNEL);

        if (!p)
                return NULL;
        if (sw_size) {
                s = kcalloc(nelem, sw_size, GFP_KERNEL);

                if (!s) {
                        dma_free_coherent(&pdev->dev, len, p, *phys);
Quoted 10 lines
                        return NULL;
                }
        }
        if (metadata)
                *(void **)metadata = s;
        memset(p, 0, len);
        return p;
}

yeah, that is a bit silly-looking.

#2 lore raw
Quoted 7 lines

There will be a memory leak in the function alloc_ring when the parameter sw_size is nonzero and metadata is NULL. I don't know if that can happen.

Here is the code, taken from the latest git tree.

static void *alloc_ring(struct pci_dev *pdev, size_t nelem, size_t elem_size,
size_t sw_size, dma_addr_t * phys, void *metadata)

It might make sense to stick a WARN_ON(sw_size && !metadata) in that function or something like that, but a quick audit of the current code (there are only 4 callers of alloc_ring() in that file -- and it's static so we know those are all of them -- all in t3_sge_alloc_qset()) shows that all callers pass non-NULL metadata if sw_size != 0.

- R.

#3 lore raw
In reply to Andrew Morton

[adding cxgb3 maintainer to cc list]

Quoted 61 lines · On Sat, 22 Nov 2008 21:35:32 -0800 Andrew Morton <[email protected]>…

(switched to email. Please respond via emailed reply-to-all, not via the bugzilla web interface).

Quoted 31 lines · On Sat, 22 Nov 2008 21:11:51 -0800 (PST) [email protected] wrote:

http://bugzilla.kernel.org/show_bug.cgi?id=12087

Summary: [drivers/net/cxgb3/sge.c:563]: Possible memory
leak: s Product: Drivers
Version: 2.5
KernelVersion: latest git tree
Platform: All
OS/Version: Linux
Tree: Mainline
Status: NEW
Severity: normal
Priority: P1
Component: Network
AssignedTo: [email protected]
ReportedBy: [email protected]

There will be a memory leak in the function alloc_ring when the parameter sw_size is nonzero and metadata is NULL. I don't know if that can happen.

Here is the code, taken from the latest git tree.

static void *alloc_ring(struct pci_dev *pdev, size_t nelem, size_t elem_size, size_t sw_size, dma_addr_t * phys, void *metadata)

{
        size_t len = nelem * elem_size;
        void *s = NULL;
        void *p = dma_alloc_coherent(&pdev->dev, len, phys,

GFP_KERNEL);

Quoted 17 lines
        if (!p)
                return NULL;
        if (sw_size) {
                s = kcalloc(nelem, sw_size, GFP_KERNEL);

                if (!s) {
                        dma_free_coherent(&pdev->dev, len, p,
*phys); return NULL;
                }
        }
        if (metadata)
                *(void **)metadata = s;
        memset(p, 0, len);
        return p;
}

yeah, that is a bit silly-looking.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#4 lore raw
Quoted 1 line · Jay Cliburn

[adding cxgb3 maintainer to cc list]

Thanks, I'll look at this.

Divy

Quoted 66 lines
Quoted 61 lines · On Sat, 22 Nov 2008 21:35:32 -0800 Andrew Morton <[email protected]>…

(switched to email. Please respond via emailed reply-to-all, not via the bugzilla web interface).

Quoted 31 lines · On Sat, 22 Nov 2008 21:11:51 -0800 (PST) [email protected] wrote:

http://bugzilla.kernel.org/show_bug.cgi?id=12087

Summary: [drivers/net/cxgb3/sge.c:563]: Possible memory
leak: s Product: Drivers
Version: 2.5
KernelVersion: latest git tree
Platform: All
OS/Version: Linux
Tree: Mainline
Status: NEW
Severity: normal
Priority: P1
Component: Network
AssignedTo: [email protected]
ReportedBy: [email protected]

There will be a memory leak in the function alloc_ring when the parameter sw_size is nonzero and metadata is NULL. I don't know if that can happen.

Here is the code, taken from the latest git tree.

static void *alloc_ring(struct pci_dev *pdev, size_t nelem, size_t elem_size, size_t sw_size, dma_addr_t * phys, void *metadata)

{
        size_t len = nelem * elem_size;
        void *s = NULL;
        void *p = dma_alloc_coherent(&pdev->dev, len, phys,

GFP_KERNEL);

Quoted 17 lines
        if (!p)
                return NULL;
        if (sw_size) {
                s = kcalloc(nelem, sw_size, GFP_KERNEL);

                if (!s) {
                        dma_free_coherent(&pdev->dev, len, p,
*phys); return NULL;
                }
        }
        if (metadata)
                *(void **)metadata = s;
        memset(p, 0, len);
        return p;
}

yeah, that is a bit silly-looking.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#5 lore raw
In reply to Andrew Morton
Quoted 52 lines · Andrew Morton

(switched to email. Please respond via emailed reply-to-all, not via the bugzilla web interface).

Quoted 36 lines · On Sat, 22 Nov 2008 21:11:51 -0800 (PST) [email protected] wrote:

http://bugzilla.kernel.org/show_bug.cgi?id=12087

Summary: [drivers/net/cxgb3/sge.c:563]: Possible memory leak: s
Product: Drivers
Version: 2.5
KernelVersion: latest git tree
Platform: All
OS/Version: Linux
Tree: Mainline
Status: NEW
Severity: normal
Priority: P1
Component: Network
AssignedTo: [email protected]
ReportedBy: [email protected]

There will be a memory leak in the function alloc_ring when the parameter sw_size is nonzero and metadata is NULL. I don't know if that can happen.

Here is the code, taken from the latest git tree.

static void *alloc_ring(struct pci_dev *pdev, size_t nelem, size_t elem_size,
size_t sw_size, dma_addr_t * phys, void *metadata)

{
        size_t len = nelem * elem_size;
        void *s = NULL;
        void *p = dma_alloc_coherent(&pdev->dev, len, phys, GFP_KERNEL);

        if (!p)
                return NULL;
        if (sw_size) {
                s = kcalloc(nelem, sw_size, GFP_KERNEL);

                if (!s) {
                        dma_free_coherent(&pdev->dev, len, p, *phys);
Quoted 10 lines
                        return NULL;
                }
        }
        if (metadata)
                *(void **)metadata = s;
        memset(p, 0, len);
        return p;
}
Quoted 1 line

yeah, that is a bit silly-looking.

The following patch should hopefully take care of this issue:
http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=commitdiff;h=5256554489531f3e177e7308752d8f0681cdd5a6

Cheers,
Divy