From: "Vladimir 'φ-coder/phcoder' Serbinenko" <phcoder@gmail.com>
To: linux-kernel@vger.kernel.org
Subject: [PATCH] Fix race condition in AFFS
Date: Tue, 08 May 2012 23:28:33 +0200 [thread overview]
Message-ID: <4FA99001.8080800@gmail.com> (raw)
[-- Attachment #1.1: Type: text/plain, Size: 413 bytes --]
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: 3421 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 --]
next reply other threads:[~2012-05-08 21:28 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-05-08 21:28 Vladimir 'φ-coder/phcoder' Serbinenko [this message]
2012-05-09 22:01 ` Fwd: [PATCH] Fix race condition in AFFS Vladimir 'φ-coder/phcoder' Serbinenko
2012-05-10 14:23 ` Jan Kara
2012-05-12 0:06 ` Vladimir 'φ-coder/phcoder' Serbinenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4FA99001.8080800@gmail.com \
--to=phcoder@gmail.com \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.