Bug 255874 - netgraph: Fix a double free in ng_checksum_rcvdata
Summary: netgraph: Fix a double free in ng_checksum_rcvdata
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: Lutz Donnerhacke
URL: https://reviews.freebsd.org/D30273
Keywords:
Depends on:
Blocks:
 
Reported: 2021-05-14 12:52 UTC by lylgood
Modified: 2021-05-24 03:05 UTC (History)
2 users (show)

See Also:
koobs: mfc-stable13+
koobs: mfc-stable12+
koobs: mfc-stable11+


Attachments
removes the NG_FREE_M(m) from drop branch. (358 bytes, patch)
2021-05-14 12:52 UTC, lylgood
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description lylgood 2021-05-14 12:52:45 UTC
Created attachment 224937 [details]
removes the NG_FREE_M(m) from drop branch.

Bug File: sys/netgraph/ng_checksum.c

In function ng_checksum_rcvdata, it calls checksum_ipv4(priv, m, pullup_len) and checksum_ipv6(priv, m, pullup_len). Inside these callees, macro PULLUP_CHECK is called. According the definition of this macro, m could be freed in m_pullup() and  return ENOBUFS.

Then caller ng_checksum_rcvdata accept the ENOBUFS and goto drop branch, where the freed m is freed again by NG_FREE_M() at line 687.

My patch removes the NG_FREE_M(m) from drop branch.
Comment 1 Lutz Donnerhacke freebsd_committer freebsd_triage 2021-05-15 09:41:21 UTC
Thank you for you detection, analysis and fix of this bug.

Change is now under review D30273
I'll wait for someone else to review this independently.
If your bug report had been a review, I'v simply accepted it.
Comment 2 commit-hook freebsd_committer freebsd_triage 2021-05-16 17:41:51 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=687e510e5ce32fddf46a9dc1d517ccc8a8e25581

commit 687e510e5ce32fddf46a9dc1d517ccc8a8e25581
Author:     Lutz Donnerhacke <donner@FreeBSD.org>
AuthorDate: 2021-05-15 09:32:57 +0000
Commit:     Lutz Donnerhacke <donner@FreeBSD.org>
CommitDate: 2021-05-16 17:39:51 +0000

    netgraph/ng_checksum: Fix double free error

    m_pullup(9) frees the mbuf(9) chain in the case of an allocation error.
    The mbuf chain must not be freed again in this case.

    PR:             255874
    Submitted by:   <lylgood@foxmail.com>
    Approved by:    markj
    MFC after:      1 week
    Differential Revision: https://reviews.freebsd.org/D30273

 sys/netgraph/ng_checksum.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 3 commit-hook freebsd_committer freebsd_triage 2021-05-23 12:56:50 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=fa670efa25ad960e17a6a9cb4601e5c3f19de5da

commit fa670efa25ad960e17a6a9cb4601e5c3f19de5da
Author:     Lutz Donnerhacke <donner@FreeBSD.org>
AuthorDate: 2021-05-15 09:32:57 +0000
Commit:     Lutz Donnerhacke <donner@FreeBSD.org>
CommitDate: 2021-05-23 12:55:20 +0000

    netgraph/ng_checksum: Fix double free error

    m_pullup(9) frees the mbuf(9) chain in the case of an allocation error.
    The mbuf chain must not be freed again in this case.

    PR:             255874
    Submitted by:   <lylgood@foxmail.com>
    Approved by:    markj
    Differential Revision: https://reviews.freebsd.org/D30273

    (cherry picked from commit 687e510e5ce32fddf46a9dc1d517ccc8a8e25581)

 sys/netgraph/ng_checksum.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 4 commit-hook freebsd_committer freebsd_triage 2021-05-23 12:59:57 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=76b96a4ec7fa8cffbfe8e876d622fd4e69f25267

commit 76b96a4ec7fa8cffbfe8e876d622fd4e69f25267
Author:     Lutz Donnerhacke <donner@FreeBSD.org>
AuthorDate: 2021-05-15 09:32:57 +0000
Commit:     Lutz Donnerhacke <donner@FreeBSD.org>
CommitDate: 2021-05-23 12:59:28 +0000

    netgraph/ng_checksum: Fix double free error

    m_pullup(9) frees the mbuf(9) chain in the case of an allocation error.
    The mbuf chain must not be freed again in this case.

    PR:             255874
    Submitted by:   <lylgood@foxmail.com>
    Approved by:    markj
    Differential Revision: https://reviews.freebsd.org/D30273

    (cherry picked from commit 687e510e5ce32fddf46a9dc1d517ccc8a8e25581)

 sys/netgraph/ng_checksum.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 5 commit-hook freebsd_committer freebsd_triage 2021-05-23 13:02:59 UTC
A commit in branch stable/11 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=6bc3535519f7206f844c3ffd0ee282e8875dceb4

commit 6bc3535519f7206f844c3ffd0ee282e8875dceb4
Author:     Lutz Donnerhacke <donner@FreeBSD.org>
AuthorDate: 2021-05-15 09:32:57 +0000
Commit:     Lutz Donnerhacke <donner@FreeBSD.org>
CommitDate: 2021-05-23 13:01:34 +0000

    netgraph/ng_checksum: Fix double free error

    m_pullup(9) frees the mbuf(9) chain in the case of an allocation error.
    The mbuf chain must not be freed again in this case.

    PR:             255874
    Submitted by:   <lylgood@foxmail.com>
    Approved by:    markj
    Differential Revision: https://reviews.freebsd.org/D30273

    (cherry picked from commit 687e510e5ce32fddf46a9dc1d517ccc8a8e25581)

 sys/netgraph/ng_checksum.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)