Fix race condition in catopen(3).
ClosedPublic
Actions

Authored by delphij on Mar 17 2020, 6:44 AM.

Details

Reviewers

emaste
henry.hu.sh_gmail.com
markj

Commits

rS359118: Fix race condition in catopen(3).

Summary

Fix race condition in catopen(3).

The current code uses a rwlock to protect the cached list, which
in turn holds a list of catentry objects, and increments reference
count while holding only read lock.

Fix this by converting the reference counter to use atomic operations.

While I'm there, also perform some clean ups around memory operations.

PR: 202636
Reported by: Henry Hu <henry.hu.sh@gmail.com>

Test Plan

Run test case provided in the bug.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 29991
Build 27804: arc lint + arc unit

Event Timeline

delphij created this revision.Mar 17 2020, 6:44 AM

Herald added a subscriber: imp. · View Herald TranscriptMar 17 2020, 6:44 AM

Harbormaster completed remote builds in B29974: Diff 69587.Mar 17 2020, 6:44 AM

delphij added a reviewer: henry.hu.sh_gmail.com.Mar 17 2020, 6:44 AM

emaste added a reviewer: markj.Mar 17 2020, 4:31 PM

markj added inline comments.Mar 17 2020, 6:45 PM

lib/libc/nls/msgcat.c
162	All of the `refcount` field references use unneeded parentheses.
366	Why not use atomic_fetchadd_int()?

Address reviewer comments.

Harbormaster completed remote builds in B29991: Diff 69628.Mar 18 2020, 4:15 AM

Please take another look.

Seems ok to me. I am skeptical that a rw lock is better than a mutex here.

lib/libc/nls/msgcat.c
86	You might explicitly initialize the refcount to 0 here with a short comment explaining why negative entries don't have any references, but it is up to you.

This revision is now accepted and ready to land.Mar 18 2020, 5:08 PM

Closed by commit rS359118: Fix race condition in catopen(3). (authored by delphij). · Explain WhyMar 19 2020, 6:33 AM

This revision was automatically updated to reflect the committed changes.

delphij added a commit: rS359118: Fix race condition in catopen(3)..