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 Not Applicable

Unit

Tests Not Applicable

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 ↗	(On Diff #69587)	All of the `refcount` field references use unneeded parentheses.
366 ↗	(On Diff #69587)	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 ↗	(On Diff #69628)	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.