Bug 255866 - [PATCH] dev/ocs_fc: Fix a use after free in ocs_sport_free
Summary: [PATCH] dev/ocs_fc: Fix a use after free in ocs_sport_free
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: Ram Kishore Vegesna
Depends on:
Reported: 2021-05-14 11:00 UTC by lylgood
Modified: 2021-05-31 06:24 UTC (History)
3 users (show)

See Also:

add a new variable ocs (864 bytes, patch)
2021-05-14 11:00 UTC, lylgood
no flags Details | Diff
Use ocs from sport. (417 bytes, text/plain)
2021-05-27 05:56 UTC, Ram Kishore Vegesna
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description lylgood 2021-05-14 11:00:56 UTC
Created attachment 224930 [details]
add a new variable ocs

Bug File: sys/dev/ocs_fc/ocs_sport.c

In function ocs_sport_free, domain could be freed via ocs_domain_post_event(domain,..)->ocs_domain_free(domain)->ocs_free(ocs, domain, sizeof(*domain)). 

But the freed domain is still used by domain->ocs at line 266 via ocs_free(domain->ocs, sport, sizeof(*sport)).

My patch adds a new variable ocs and sets ocs =domain->ocs before domain can be free, to avoid the uaf bug.
Comment 1 Mark Johnston freebsd_committer 2021-05-26 14:04:16 UTC
Ram, could you please take a look at this and PR 255865?
Comment 2 Ram Kishore Vegesna freebsd_committer 2021-05-27 05:56:00 UTC
Created attachment 225300 [details]
Use ocs from sport.
Comment 3 Ram Kishore Vegesna freebsd_committer 2021-05-27 06:01:57 UTC
Thanks for reporting the issue. I think the attached patch will avoid new variable.

Please let me know if you want me to check-in the changes.
Comment 4 lylgood 2021-05-27 07:49:17 UTC
(In reply to Ram Kishore Vegesna from comment #3)

Yes, i think your patch is better.
Comment 5 Mark Johnston freebsd_committer 2021-05-27 13:18:01 UTC
(In reply to Ram Kishore Vegesna from comment #3)
Thanks for the quick reply.  Yes, please go ahead and commit.  Be sure to include "PR:" tags in the commit log message.
Comment 6 commit-hook freebsd_committer 2021-05-28 05:54:33 UTC
A commit in branch main references this bug:

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

commit dd722ccd6efcaed5c6056e51a044db7f1a5b4a0d
Author:     Ram Kishore Vegesna <ram@FreeBSD.org>
AuthorDate: 2021-05-28 05:26:13 +0000
Commit:     Ram Kishore Vegesna <ram@FreeBSD.org>
CommitDate: 2021-05-28 05:26:13 +0000

    ocs_fc: Fix a use after free in ocs_sport_free

    Domain which could be freed is used while freeing the sport.
    Use ocs from sport.

    PR: 255866
    Reported by: lylgood@foxmail.com
    Approved by:: markj

 sys/dev/ocs_fc/ocs_sport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 7 dave 2021-05-30 18:26:20 UTC
Are there plans to backport this patch to stable/12?
Comment 8 Ram Kishore Vegesna freebsd_committer 2021-05-31 06:24:31 UTC
(In reply to dave from comment #7)
As of now, I don't have plans to backport this change. Will merge if any customer is facing the issue.