Bugzilla – Bug 387055
VUL-0: CVE-2008-1669: Kernel: add rcu_read_lock() to fs/locks.c and fix fcntl store/load
Last modified: 2021-08-11 09:13:36 UTC
From: "Jan Lieskovsky (RH kernel Security Response Team)" <jlieskov@redhat.com> To: vendor-sec@lst.de Cc: Tomas Hoger <thoger@redhat.com>, Alexander Viro <aviro@redhat.com>, Linda Wang <lwang@redhat.com> ... Hello guys, Alexander Viro has discovered yet another potential security issue related with absence of protection mechanism when attempting to access critical section of code in fs/locks.c and also fixing fcntl store/load mismatch. Problem description: ==================== locks.c (fcntl_setlk()) has subtler one - we need to make sure that if we *do* have an fcntl/close race on SMP box, the access to descriptor table and inode->i_flock won't get reordered. As it is, we get STORE inode->i_flock, LOAD descriptor table entry vs. STORE descriptor table entry, LOAD inode->i_flock with not a single lock in common on both sides. We do have BKL around the first STORE, but check in locks_remove_posix() is outside of BKL and for a good reason - we don't want BKL on common path of close(2). Proposed CVE description: ======================== The absence of a protection mechanism when attempting to access a critical section of code has been found in the Linux kernel open file descriptors control mechanism, fcntl. This could allow a local unprivileged user to simultaneously execute code, which would otherwise be protected against parallel execution. As well, a race condition when handling locks in the Linux kernel fcntl functionality, may have allowed a process belonging to a local unprivileged user to gain re-ordered access to the descriptor table. Impact: ======== According to our metrics, this one reaches important level of severity. Affected kernel versions: ========================= The issue seems to be there for years. Affects all kernel versions from 2.4.19+ up to the latest. Alexander will inform the upstream with details. Testcase: ========= Unfortunately, no testcase available for this one. Proposed patch: ============== See attachment cve-2008-1669.patch Kind regards Jan iankko Lieskovsky RH kernel Security Response Team
Created attachment 212551 [details] patch from the mail ...
From: Alexander Viro <aviro@redhat.com> To: "Jan Lieskovsky (RH kernel Security Response Team)" <jlieskov@redhat.com> Cc: vendor-sec@lst.de, Tomas Hoger <thoger@redhat.com>, Alexander Viro <aviro@redhat.com>, Linda Wang <lwang@redhat.com> On Mon, May 05, 2008 at 08:31:28PM +0200, Jan Lieskovsky (RH kernel Security +Response Team) wrote: > The issue seems to be there for years. Affects all kernel versions from > 2.4.19+ up to the latest. Alexander will inform the upstream with > details. Oh, for... 2.4.19+ is dnotify crap. This one is 2.6; it fixes a hole in a partial fix for race that apparently is still wide-open in 2.4 (i.e. even the partial fix is absent). In 2.4 you just need to take a lock on file from task A, have task B block on trying to do the same, then have task C (sharing the descriptor table with B) close the file B is trying to lock. Then have A drop its lock and (2.4) box is well and truly fucked. To be honest, I simply assumed that this crap _was_ backported to 2.4 at the same point when it went to 2.6. Alas, it wasn't.
affects _all_ kernel versions
err, 2.6 I mean ;)
Created attachment 212939 [details] another patch The vendor-sec mails are pretty strange. Aviro postes another fix inline in his mail. I am attaching it in the hope its usefull.
Thanks for forwarding. As far as I understand comment in his email (and looking into the patch), this second patch is backport to 2.4 kernels. So I'm going to commit to SLES10 SP1 and SLES10 SP2 the patch from comment #1.
please do not forget the sles9, 10.2 and 10.3 trees. Not sure if we should backport to sles8 (2.4), but so far I dont think its necessary.
upstream git commit id is 0b2bac2f1ea0d33a3621b27ca68b9ae760fca2e9
Committed the patch to SLES10 SP1, 10.2, 10.3 and SLES10 SP2 trees (there the patch is disabled and will be enabled after SP2 is released so that it goes into the maintenance update as discussed with Jiri Kosina and Marcus). The fix isn't needed for SLES9 kernels because they use file_lock instead of RCU and so reordering cannot happen. Closing the bug.
10.2 and 10.3 kernels released
This bug is mentioned/fixed in released sles/sled 10 SP1 kernel updates on July 3rd 2008, version 2.6.16.54-0.2.8
This bug was mentioned / fixed in the SLE 10 SP2 kernel, version 2.6.16.60-0.25, released on Fri 18 July 2008.
Due to a regression, we had to rerelease the kernel update for SLE 10 SP2. The update was released as 2.6.16.60-0.27 kernel on Tue, 29th of July 2008.
Can someone verify if this bug really affects SLES8? If it does, I have a customer with an extended SLES8 support contract who wants a backport of the fix (SWAMP will be opened as soon as I know if SLES8 is affected)
Patch from comment #5 applies almost cleanly (with some offsets and one small reject) to SLES8 sources. I will try to apply it to newest maintenance update sources for SLES8. Anders, what archs will you need the PTFs for?
See comment #2: I think they will need the full fix. It seems we will need the initial part of the fix as well, since according to #2, it's not there Architectures: x86_64 and x86, k_dflt and k_smp
The fix attached to this bug in comment #5 should fix all the races wrt. fcntl_setlk() and close() which are described in comment #2. So as far as I can see that fix should be enough.
Apparently, x86_64 is no longer supported for SLES-8, the only architectures with extended support are S/390 and i386 (see http://fourier.suse.de/mlarch/SuSE/devel/2007/devel.2007.11/msg00006.html). Packages for k_deflt and k_smp for sles8-i386 are building now.
Good to know about SLES8, I didn't know that x86_64 was dead. Thanks for info
Hmm, it won't be as easy as it looks. locks.c: In function `fcntl_setlk': locks.c:1665: structure has no member named `fl_type' locks.c: In function `fcntl_setlk64': locks.c:1808: structure has no member named `fl_type'
Very similar line in both occasions... now I'm not sure whether it's supposed to be flock.l_type or file_lock.fl_type. if (!error && flock.fl_type != F_UNLCK && filp != f) {
I think that the correct one is flock.l_type, because that's what is requested initially by caller of this function. If we want to unlock, we don't care about any race, we're going to drop the lock anyway. But, because this file_lock is just gained by conversion earlier: flock_to_posix_lock(filp, file_lock, &flock); Maybe it doesn't matter which one we use?
As far as I read the code it should not matter which one we really use but flock.l_type fits more the logic of the function so I'd use that one for checking.
Created attachment 240831 [details] final patch used in SLES8 PTF
Committed to SLES8 tree.
CVE-2008-1669: CVSS v2 Base Score: 6.9 (AV:L/AC:M/Au:N/C:C/I:C/A:C)