* Fwd: [PATCH] Fix race condition in AFFS
[not found] <4FA99001.8080800@gmail.com>
@ 2012-05-09 22:01 ` Vladimir 'φ-coder/phcoder' Serbinenko
[not found] ` <20120510142300.GB4343@quack.suse.cz>
1 sibling, 0 replies; 2+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2012-05-09 22:01 UTC (permalink / raw)
To: linux-fsdevel
[-- Attachment #1.1: Type: text/plain, Size: 712 bytes --]
Since it was ignored on LKML, I guess this is the right list.
-------- Original Message --------
Subject: [PATCH] Fix race condition in AFFS
Date: Tue, 08 May 2012 23:28:33 +0200
From: Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>
To: linux-kernel@vger.kernel.org
AFFS preallocates few blocks per inode as an optimisation. Unfortunately
there is a race condition and the same blocks ends up being allocated
for both data and metadata, metadata gets overwritten and so the file is
partially unreadable. Here is a fix.
Please indicate if this isn't the right place to submit this or my
previous fs-related patches.
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: affs-race.diff --]
[-- Type: text/x-diff; name="affs-race.diff", Size: 3423 bytes --]
=== modified file 'affs.h'
--- affs.h 2012-05-07 16:42:19 +0000
+++ affs.h 2012-05-08 21:24:08 +0000
@@ -66,8 +66,11 @@ struct affs_inode_info {
struct buffer_head *i_ext_bh; /* bh of last extended block */
loff_t mmu_private;
u32 i_protect; /* unused attribute bits */
+
u32 i_lastalloc; /* last allocated block */
int i_pa_cnt; /* number of preallocated blocks */
+ struct mutex i_alloc; /* Protects last 2 fields. */
+
struct inode vfs_inode;
};
=== modified file 'bitmap.c'
--- bitmap.c 2012-05-06 21:40:49 +0000
+++ bitmap.c 2012-05-08 21:24:08 +0000
@@ -151,12 +151,18 @@ affs_alloc_block(struct inode *inode, u3
pr_debug("AFFS: balloc(inode=%lu,goal=%u): ", inode->i_ino, goal);
+ mutex_lock(&AFFS_I (inode)->i_alloc);
+
if (AFFS_I(inode)->i_pa_cnt) {
- pr_debug("%d\n", AFFS_I(inode)->i_lastalloc+1);
+ u32 ret;
AFFS_I(inode)->i_pa_cnt--;
- return ++AFFS_I(inode)->i_lastalloc;
+ ret = ++AFFS_I(inode)->i_lastalloc;
+ mutex_unlock(&AFFS_I (inode)->i_alloc);
+ return ret;
}
+ mutex_unlock(&AFFS_I (inode)->i_alloc);
+
if (!goal || goal > sbi->s_partition_size) {
if (goal)
affs_warning(sb, "affs_balloc", "invalid goal %d", goal);
@@ -230,16 +236,22 @@ find_bit:
bit = ffs(tmp & mask) - 1;
blk += bit + sbi->s_reserved;
mask2 = mask = 1 << (bit & 31);
- AFFS_I(inode)->i_lastalloc = blk;
- /* prealloc as much as possible within this word */
- while ((mask2 <<= 1)) {
- if (!(tmp & mask2))
- break;
- AFFS_I(inode)->i_pa_cnt++;
- mask |= mask2;
+ mutex_lock(&AFFS_I (inode)->i_alloc);
+ if (!AFFS_I(inode)->i_pa_cnt) {
+ AFFS_I(inode)->i_lastalloc = blk;
+
+ /* prealloc as much as possible within this word */
+ while ((mask2 <<= 1)) {
+ if (!(tmp & mask2))
+ break;
+ AFFS_I(inode)->i_pa_cnt++;
+ mask |= mask2;
+ }
+ bm->bm_free -= AFFS_I(inode)->i_pa_cnt + 1;
}
- bm->bm_free -= AFFS_I(inode)->i_pa_cnt + 1;
+
+ mutex_unlock(&AFFS_I (inode)->i_alloc);
*data = cpu_to_be32(tmp & ~mask);
=== modified file 'file.c'
--- file.c 2012-05-06 21:40:49 +0000
+++ file.c 2012-05-08 21:24:08 +0000
@@ -795,12 +795,21 @@ void
affs_free_prealloc(struct inode *inode)
{
struct super_block *sb = inode->i_sb;
+ u32 first, cnt;
pr_debug("AFFS: free_prealloc(ino=%lu)\n", inode->i_ino);
- while (AFFS_I(inode)->i_pa_cnt) {
- AFFS_I(inode)->i_pa_cnt--;
- affs_free_block(sb, ++AFFS_I(inode)->i_lastalloc);
+ mutex_lock(&AFFS_I (inode)->i_alloc);
+ first = AFFS_I(inode)->i_lastalloc;
+ cnt = AFFS_I(inode)->i_pa_cnt;
+ AFFS_I(inode)->i_lastalloc += cnt;
+ AFFS_I(inode)->i_pa_cnt = 0;
+
+ mutex_unlock(&AFFS_I (inode)->i_alloc);
+
+ while (cnt) {
+ cnt--;
+ affs_free_block(sb, ++first);
}
}
=== modified file 'inode.c'
--- inode.c 2012-05-07 16:23:07 +0000
+++ inode.c 2012-05-08 21:24:08 +0000
@@ -70,6 +70,7 @@ struct inode *affs_iget(struct super_blo
AFFS_I(inode)->mmu_private = 0;
AFFS_I(inode)->i_lastalloc = 0;
AFFS_I(inode)->i_pa_cnt = 0;
+ mutex_init(&AFFS_I (inode)->i_alloc);
if (sbi->s_flags & SF_SETMODE)
inode->i_mode = sbi->s_mode;
@@ -326,6 +327,7 @@ affs_new_inode(struct inode *dir)
AFFS_I(inode)->i_pa_cnt = 0;
AFFS_I(inode)->i_extcnt = 1;
AFFS_I(inode)->i_ext_last = ~1;
+ mutex_init(&AFFS_I (inode)->i_alloc);
insert_inode_hash(inode);
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread