* [PATCH 1/2] tmpfs: fix use-after-free of mempolicy object
@ 2013-02-20 7:11 Greg Thelen
2013-02-20 7:11 ` [PATCH 2/2] tmpfs: fix mempolicy object leaks Greg Thelen
2013-02-20 20:21 ` [PATCH 1/2] tmpfs: fix use-after-free of mempolicy object Hugh Dickins
0 siblings, 2 replies; 8+ messages in thread
From: Greg Thelen @ 2013-02-20 7:11 UTC (permalink / raw)
To: Hugh Dickins; +Cc: akpm, linux-mm, linux-kernel, Greg Thelen
The tmpfs remount logic preserves filesystem mempolicy if the mpol=M
option is not specified in the remount request. A new policy can be
specified if mpol=M is given.
Before this patch remounting an mpol bound tmpfs without specifying
mpol= mount option in the remount request would set the filesystem's
mempolicy object to a freed mempolicy object.
To reproduce the problem boot a DEBUG_PAGEALLOC kernel and run:
# mkdir /tmp/x
# mount -t tmpfs -o size=100M,mpol=interleave nodev /tmp/x
# grep /tmp/x /proc/mounts
nodev /tmp/x tmpfs rw,relatime,size=102400k,mpol=interleave:0-3 0 0
# mount -o remount,size=200M nodev /tmp/x
# grep /tmp/x /proc/mounts
nodev /tmp/x tmpfs rw,relatime,size=204800k,mpol=??? 0 0
# note ? garbage in mpol=... output above
# dd if=/dev/zero of=/tmp/x/f count=1
# panic here
Panic:
BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [< (null)>] (null)
[...]
Oops: 0010 [#1] SMP DEBUG_PAGEALLOC
Call Trace:
[<ffffffff81186ead>] ? mpol_set_nodemask+0x8d/0x100
[<ffffffff811895ef>] ? mpol_shared_policy_init+0x8f/0x160
[<ffffffff81189605>] mpol_shared_policy_init+0xa5/0x160
[<ffffffff811580e1>] ? shmem_get_inode+0x1e1/0x270
[<ffffffff811580e1>] ? shmem_get_inode+0x1e1/0x270
[<ffffffff810db15d>] ? trace_hardirqs_on+0xd/0x10
[<ffffffff81158109>] shmem_get_inode+0x209/0x270
[<ffffffff811581ae>] shmem_mknod+0x3e/0xf0
[<ffffffff811582b8>] shmem_create+0x18/0x20
[<ffffffff811af5d5>] vfs_create+0xb5/0x130
[<ffffffff811afff1>] do_last+0x9a1/0xea0
[<ffffffff811ac77a>] ? link_path_walk+0x7a/0x930
[<ffffffff811b05a3>] path_openat+0xb3/0x4d0
[<ffffffff811be831>] ? __alloc_fd+0x31/0x160
[<ffffffff811b0de2>] do_filp_open+0x42/0xa0
[<ffffffff811be8e0>] ? __alloc_fd+0xe0/0x160
[<ffffffff811a066e>] do_sys_open+0xfe/0x1e0
[<ffffffff811f0aeb>] compat_sys_open+0x1b/0x20
[<ffffffff815d6055>] cstar_dispatch+0x7/0x1f
Non-debug kernels will not crash immediately because referencing the
dangling mpol will not cause a fault. Instead the filesystem will
reference a freed mempolicy object, which will cause unpredictable
behavior.
The problem boils down to a dropped mpol reference below if
shmem_parse_options() does not allocate a new mpol:
config = *sbinfo
shmem_parse_options(data, &config, true)
mpol_put(sbinfo->mpol)
sbinfo->mpol = config.mpol /* BUG: saves unreferenced mpol */
This patch avoids the crash by not releasing the mempolicy if
shmem_parse_options() doesn't create a new mpol.
How far back does this issue go? I see it in both 2.6.36 and 3.3. I
did not look back further.
Signed-off-by: Greg Thelen <gthelen@google.com>
---
mm/shmem.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/mm/shmem.c b/mm/shmem.c
index 5dd56f6..efd0b3a 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2487,6 +2487,7 @@ static int shmem_remount_fs(struct super_block *sb, int *flags, char *data)
unsigned long inodes;
int error = -EINVAL;
+ config.mpol = NULL;
if (shmem_parse_options(data, &config, true))
return error;
@@ -2511,8 +2512,13 @@ static int shmem_remount_fs(struct super_block *sb, int *flags, char *data)
sbinfo->max_inodes = config.max_inodes;
sbinfo->free_inodes = config.max_inodes - inodes;
- mpol_put(sbinfo->mpol);
- sbinfo->mpol = config.mpol; /* transfers initial ref */
+ /*
+ * Preserve previous mempolicy unless mpol remount option was specified.
+ */
+ if (config.mpol) {
+ mpol_put(sbinfo->mpol);
+ sbinfo->mpol = config.mpol; /* transfers initial ref */
+ }
out:
spin_unlock(&sbinfo->stat_lock);
return error;
--
1.8.1.3
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] tmpfs: fix mempolicy object leaks
2013-02-20 7:11 [PATCH 1/2] tmpfs: fix use-after-free of mempolicy object Greg Thelen
@ 2013-02-20 7:11 ` Greg Thelen
2013-02-20 20:26 ` Hugh Dickins
2013-02-20 20:21 ` [PATCH 1/2] tmpfs: fix use-after-free of mempolicy object Hugh Dickins
1 sibling, 1 reply; 8+ messages in thread
From: Greg Thelen @ 2013-02-20 7:11 UTC (permalink / raw)
To: Hugh Dickins; +Cc: akpm, linux-mm, linux-kernel, Greg Thelen
This patch fixes several mempolicy leaks in the tmpfs mount logic.
These leaks are slow - on the order of one object leaked per mount
attempt.
Leak 1 (umount doesn't free mpol allocated in mount):
while true; do
mount -t tmpfs -o mpol=interleave,size=100M nodev /mnt
umount /mnt
done
Leak 2 (errors parsing remount options will leak mpol):
mount -t tmpfs -o size=100M nodev /mnt
while true; do
mount -o remount,mpol=interleave,size=x /mnt 2> /dev/null
done
umount /mnt
Leak 3 (multiple mpol per mount leak mpol):
while true; do
mount -t tmpfs -o mpol=interleave,mpol=interleave,size=100M nodev /mnt
umount /mnt
done
This patch fixes all of the above. I could have broken the patch into
three pieces but is seemed easier to review as one.
Signed-off-by: Greg Thelen <gthelen@google.com>
---
mm/shmem.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/mm/shmem.c b/mm/shmem.c
index efd0b3a..ed2cb26 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2386,6 +2386,7 @@ static int shmem_parse_options(char *options, struct shmem_sb_info *sbinfo,
bool remount)
{
char *this_char, *value, *rest;
+ struct mempolicy *mpol = NULL;
uid_t uid;
gid_t gid;
@@ -2414,7 +2415,7 @@ static int shmem_parse_options(char *options, struct shmem_sb_info *sbinfo,
printk(KERN_ERR
"tmpfs: No value for mount option '%s'\n",
this_char);
- return 1;
+ goto error;
}
if (!strcmp(this_char,"size")) {
@@ -2463,19 +2464,23 @@ static int shmem_parse_options(char *options, struct shmem_sb_info *sbinfo,
if (!gid_valid(sbinfo->gid))
goto bad_val;
} else if (!strcmp(this_char,"mpol")) {
- if (mpol_parse_str(value, &sbinfo->mpol))
+ mpol_put(mpol);
+ if (mpol_parse_str(value, &mpol))
goto bad_val;
} else {
printk(KERN_ERR "tmpfs: Bad mount option %s\n",
this_char);
- return 1;
+ goto error;
}
}
+ sbinfo->mpol = mpol;
return 0;
bad_val:
printk(KERN_ERR "tmpfs: Bad value '%s' for mount option '%s'\n",
value, this_char);
+error:
+ mpol_put(mpol);
return 1;
}
@@ -2551,6 +2556,7 @@ static void shmem_put_super(struct super_block *sb)
struct shmem_sb_info *sbinfo = SHMEM_SB(sb);
percpu_counter_destroy(&sbinfo->used_blocks);
+ mpol_put(sbinfo->mpol);
kfree(sbinfo);
sb->s_fs_info = NULL;
}
--
1.8.1.3
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] tmpfs: fix use-after-free of mempolicy object
2013-02-20 7:11 [PATCH 1/2] tmpfs: fix use-after-free of mempolicy object Greg Thelen
2013-02-20 7:11 ` [PATCH 2/2] tmpfs: fix mempolicy object leaks Greg Thelen
@ 2013-02-20 20:21 ` Hugh Dickins
1 sibling, 0 replies; 8+ messages in thread
From: Hugh Dickins @ 2013-02-20 20:21 UTC (permalink / raw)
To: Greg Thelen; +Cc: akpm, linux-mm, linux-kernel
On Tue, 19 Feb 2013, Greg Thelen wrote:
> The tmpfs remount logic preserves filesystem mempolicy if the mpol=M
> option is not specified in the remount request. A new policy can be
> specified if mpol=M is given.
>
> Before this patch remounting an mpol bound tmpfs without specifying
> mpol= mount option in the remount request would set the filesystem's
> mempolicy object to a freed mempolicy object.
>
> To reproduce the problem boot a DEBUG_PAGEALLOC kernel and run:
> # mkdir /tmp/x
>
> # mount -t tmpfs -o size=100M,mpol=interleave nodev /tmp/x
>
> # grep /tmp/x /proc/mounts
> nodev /tmp/x tmpfs rw,relatime,size=102400k,mpol=interleave:0-3 0 0
>
> # mount -o remount,size=200M nodev /tmp/x
>
> # grep /tmp/x /proc/mounts
> nodev /tmp/x tmpfs rw,relatime,size=204800k,mpol=??? 0 0
> # note ? garbage in mpol=... output above
>
> # dd if=/dev/zero of=/tmp/x/f count=1
> # panic here
>
> Panic:
> BUG: unable to handle kernel NULL pointer dereference at (null)
> IP: [< (null)>] (null)
> [...]
> Oops: 0010 [#1] SMP DEBUG_PAGEALLOC
> Call Trace:
> [<ffffffff81186ead>] ? mpol_set_nodemask+0x8d/0x100
> [<ffffffff811895ef>] ? mpol_shared_policy_init+0x8f/0x160
> [<ffffffff81189605>] mpol_shared_policy_init+0xa5/0x160
> [<ffffffff811580e1>] ? shmem_get_inode+0x1e1/0x270
> [<ffffffff811580e1>] ? shmem_get_inode+0x1e1/0x270
> [<ffffffff810db15d>] ? trace_hardirqs_on+0xd/0x10
> [<ffffffff81158109>] shmem_get_inode+0x209/0x270
> [<ffffffff811581ae>] shmem_mknod+0x3e/0xf0
> [<ffffffff811582b8>] shmem_create+0x18/0x20
> [<ffffffff811af5d5>] vfs_create+0xb5/0x130
> [<ffffffff811afff1>] do_last+0x9a1/0xea0
> [<ffffffff811ac77a>] ? link_path_walk+0x7a/0x930
> [<ffffffff811b05a3>] path_openat+0xb3/0x4d0
> [<ffffffff811be831>] ? __alloc_fd+0x31/0x160
> [<ffffffff811b0de2>] do_filp_open+0x42/0xa0
> [<ffffffff811be8e0>] ? __alloc_fd+0xe0/0x160
> [<ffffffff811a066e>] do_sys_open+0xfe/0x1e0
> [<ffffffff811f0aeb>] compat_sys_open+0x1b/0x20
> [<ffffffff815d6055>] cstar_dispatch+0x7/0x1f
>
> Non-debug kernels will not crash immediately because referencing the
> dangling mpol will not cause a fault. Instead the filesystem will
> reference a freed mempolicy object, which will cause unpredictable
> behavior.
>
> The problem boils down to a dropped mpol reference below if
> shmem_parse_options() does not allocate a new mpol:
> config = *sbinfo
> shmem_parse_options(data, &config, true)
> mpol_put(sbinfo->mpol)
> sbinfo->mpol = config.mpol /* BUG: saves unreferenced mpol */
>
> This patch avoids the crash by not releasing the mempolicy if
> shmem_parse_options() doesn't create a new mpol.
This looks very good to me, thank you Greg. I did start to wonder
about the consequence of an error in shmem_parse_options() after
allocating mempolicy, but you're ahead of me, and have sensibly
deferred leak fixes to 2/2.
>
> How far back does this issue go? I see it in both 2.6.36 and 3.3. I
> did not look back further.
It probably goes back to 2.6.26, when the refcounting came in at this
mount level. But there's been such a sad history of bugs in mempolicy
refcounting, occasionally one bug has protected against another.
And whether anyone actually uses these tmpfs mpol= options is another
question. 2.6.26 claimed to add support for "mpol=local", yet nobody
noticed that it was rejected as an invalid option, until an independent
bugfix in 2.6.34 rearranged the code and happened to get it working.
I only discovered that a month or two ago, when working to fix the
corruption which 2.6.35 then immediately bestowed upon "mpol=local".
>
> Signed-off-by: Greg Thelen <gthelen@google.com>
Acked-by: Hugh Dickins <hughd@google.com>
I'm inclined to add
Cc: stable@vger.kernel.org
to this one, but not to the 2/2 leak one; but I'm perfectly
happy if Andrew chooses differently when he picks them up.
> ---
> mm/shmem.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 5dd56f6..efd0b3a 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -2487,6 +2487,7 @@ static int shmem_remount_fs(struct super_block *sb, int *flags, char *data)
> unsigned long inodes;
> int error = -EINVAL;
>
> + config.mpol = NULL;
> if (shmem_parse_options(data, &config, true))
> return error;
>
> @@ -2511,8 +2512,13 @@ static int shmem_remount_fs(struct super_block *sb, int *flags, char *data)
> sbinfo->max_inodes = config.max_inodes;
> sbinfo->free_inodes = config.max_inodes - inodes;
>
> - mpol_put(sbinfo->mpol);
> - sbinfo->mpol = config.mpol; /* transfers initial ref */
> + /*
> + * Preserve previous mempolicy unless mpol remount option was specified.
> + */
> + if (config.mpol) {
> + mpol_put(sbinfo->mpol);
> + sbinfo->mpol = config.mpol; /* transfers initial ref */
> + }
> out:
> spin_unlock(&sbinfo->stat_lock);
> return error;
> --
> 1.8.1.3
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] tmpfs: fix mempolicy object leaks
2013-02-20 7:11 ` [PATCH 2/2] tmpfs: fix mempolicy object leaks Greg Thelen
@ 2013-02-20 20:26 ` Hugh Dickins
2013-02-20 21:06 ` Andrew Morton
2013-03-03 23:49 ` Will Huck
0 siblings, 2 replies; 8+ messages in thread
From: Hugh Dickins @ 2013-02-20 20:26 UTC (permalink / raw)
To: Greg Thelen; +Cc: akpm, linux-mm, linux-kernel
On Tue, 19 Feb 2013, Greg Thelen wrote:
> This patch fixes several mempolicy leaks in the tmpfs mount logic.
> These leaks are slow - on the order of one object leaked per mount
> attempt.
>
> Leak 1 (umount doesn't free mpol allocated in mount):
> while true; do
> mount -t tmpfs -o mpol=interleave,size=100M nodev /mnt
> umount /mnt
> done
>
> Leak 2 (errors parsing remount options will leak mpol):
> mount -t tmpfs -o size=100M nodev /mnt
> while true; do
> mount -o remount,mpol=interleave,size=x /mnt 2> /dev/null
> done
> umount /mnt
>
> Leak 3 (multiple mpol per mount leak mpol):
> while true; do
> mount -t tmpfs -o mpol=interleave,mpol=interleave,size=100M nodev /mnt
> umount /mnt
> done
>
> This patch fixes all of the above. I could have broken the patch into
> three pieces but is seemed easier to review as one.
Yes, I agree, and nicely fixed - but one doubt below. If you resolve
that, please add my Acked-by: Hugh Dickins <hughd@google.com>
>
> Signed-off-by: Greg Thelen <gthelen@google.com>
> ---
> mm/shmem.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/mm/shmem.c b/mm/shmem.c
> index efd0b3a..ed2cb26 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -2386,6 +2386,7 @@ static int shmem_parse_options(char *options, struct shmem_sb_info *sbinfo,
> bool remount)
> {
> char *this_char, *value, *rest;
> + struct mempolicy *mpol = NULL;
> uid_t uid;
> gid_t gid;
>
> @@ -2414,7 +2415,7 @@ static int shmem_parse_options(char *options, struct shmem_sb_info *sbinfo,
> printk(KERN_ERR
> "tmpfs: No value for mount option '%s'\n",
> this_char);
> - return 1;
> + goto error;
> }
>
> if (!strcmp(this_char,"size")) {
> @@ -2463,19 +2464,23 @@ static int shmem_parse_options(char *options, struct shmem_sb_info *sbinfo,
> if (!gid_valid(sbinfo->gid))
> goto bad_val;
> } else if (!strcmp(this_char,"mpol")) {
> - if (mpol_parse_str(value, &sbinfo->mpol))
> + mpol_put(mpol);
I haven't tested to check, but don't we need
mpol = NULL;
here, in case the new option turns out to be bad?
> + if (mpol_parse_str(value, &mpol))
> goto bad_val;
> } else {
> printk(KERN_ERR "tmpfs: Bad mount option %s\n",
> this_char);
> - return 1;
> + goto error;
> }
> }
> + sbinfo->mpol = mpol;
> return 0;
>
> bad_val:
> printk(KERN_ERR "tmpfs: Bad value '%s' for mount option '%s'\n",
> value, this_char);
> +error:
> + mpol_put(mpol);
> return 1;
>
> }
> @@ -2551,6 +2556,7 @@ static void shmem_put_super(struct super_block *sb)
> struct shmem_sb_info *sbinfo = SHMEM_SB(sb);
>
> percpu_counter_destroy(&sbinfo->used_blocks);
> + mpol_put(sbinfo->mpol);
> kfree(sbinfo);
> sb->s_fs_info = NULL;
> }
> --
> 1.8.1.3
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] tmpfs: fix mempolicy object leaks
2013-02-20 20:26 ` Hugh Dickins
@ 2013-02-20 21:06 ` Andrew Morton
2013-03-03 23:49 ` Will Huck
1 sibling, 0 replies; 8+ messages in thread
From: Andrew Morton @ 2013-02-20 21:06 UTC (permalink / raw)
To: Hugh Dickins; +Cc: Greg Thelen, linux-mm, linux-kernel
On Wed, 20 Feb 2013 12:26:26 -0800 (PST)
Hugh Dickins <hughd@google.com> wrote:
> > @@ -2463,19 +2464,23 @@ static int shmem_parse_options(char *options, struct shmem_sb_info *sbinfo,
> > if (!gid_valid(sbinfo->gid))
> > goto bad_val;
> > } else if (!strcmp(this_char,"mpol")) {
> > - if (mpol_parse_str(value, &sbinfo->mpol))
> > + mpol_put(mpol);
>
> I haven't tested to check, but don't we need
> mpol = NULL;
> here, in case the new option turns out to be bad?
We do.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] tmpfs: fix mempolicy object leaks
2013-02-20 20:26 ` Hugh Dickins
2013-02-20 21:06 ` Andrew Morton
@ 2013-03-03 23:49 ` Will Huck
2013-03-05 19:40 ` Hugh Dickins
1 sibling, 1 reply; 8+ messages in thread
From: Will Huck @ 2013-03-03 23:49 UTC (permalink / raw)
To: Hugh Dickins; +Cc: Greg Thelen, akpm, linux-mm, linux-kernel
Hi Hugh,
On 02/21/2013 04:26 AM, Hugh Dickins wrote:
> On Tue, 19 Feb 2013, Greg Thelen wrote:
>
>> This patch fixes several mempolicy leaks in the tmpfs mount logic.
>> These leaks are slow - on the order of one object leaked per mount
>> attempt.
>>
>> Leak 1 (umount doesn't free mpol allocated in mount):
>> while true; do
>> mount -t tmpfs -o mpol=interleave,size=100M nodev /mnt
>> umount /mnt
>> done
>>
>> Leak 2 (errors parsing remount options will leak mpol):
>> mount -t tmpfs -o size=100M nodev /mnt
>> while true; do
>> mount -o remount,mpol=interleave,size=x /mnt 2> /dev/null
>> done
>> umount /mnt
>>
>> Leak 3 (multiple mpol per mount leak mpol):
>> while true; do
>> mount -t tmpfs -o mpol=interleave,mpol=interleave,size=100M nodev /mnt
>> umount /mnt
>> done
>>
>> This patch fixes all of the above. I could have broken the patch into
>> three pieces but is seemed easier to review as one.
> Yes, I agree, and nicely fixed - but one doubt below. If you resolve
> that, please add my Acked-by: Hugh Dickins <hughd@google.com>
Could you explain me why shmem has more relationship with mempolicy? It
seems that there are many codes in shmem handle mempolicy, but other
components in mm subsystem just have little.
>
>> Signed-off-by: Greg Thelen <gthelen@google.com>
>> ---
>> mm/shmem.c | 12 +++++++++---
>> 1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/shmem.c b/mm/shmem.c
>> index efd0b3a..ed2cb26 100644
>> --- a/mm/shmem.c
>> +++ b/mm/shmem.c
>> @@ -2386,6 +2386,7 @@ static int shmem_parse_options(char *options, struct shmem_sb_info *sbinfo,
>> bool remount)
>> {
>> char *this_char, *value, *rest;
>> + struct mempolicy *mpol = NULL;
>> uid_t uid;
>> gid_t gid;
>>
>> @@ -2414,7 +2415,7 @@ static int shmem_parse_options(char *options, struct shmem_sb_info *sbinfo,
>> printk(KERN_ERR
>> "tmpfs: No value for mount option '%s'\n",
>> this_char);
>> - return 1;
>> + goto error;
>> }
>>
>> if (!strcmp(this_char,"size")) {
>> @@ -2463,19 +2464,23 @@ static int shmem_parse_options(char *options, struct shmem_sb_info *sbinfo,
>> if (!gid_valid(sbinfo->gid))
>> goto bad_val;
>> } else if (!strcmp(this_char,"mpol")) {
>> - if (mpol_parse_str(value, &sbinfo->mpol))
>> + mpol_put(mpol);
> I haven't tested to check, but don't we need
> mpol = NULL;
> here, in case the new option turns out to be bad?
>
>> + if (mpol_parse_str(value, &mpol))
>> goto bad_val;
>> } else {
>> printk(KERN_ERR "tmpfs: Bad mount option %s\n",
>> this_char);
>> - return 1;
>> + goto error;
>> }
>> }
>> + sbinfo->mpol = mpol;
>> return 0;
>>
>> bad_val:
>> printk(KERN_ERR "tmpfs: Bad value '%s' for mount option '%s'\n",
>> value, this_char);
>> +error:
>> + mpol_put(mpol);
>> return 1;
>>
>> }
>> @@ -2551,6 +2556,7 @@ static void shmem_put_super(struct super_block *sb)
>> struct shmem_sb_info *sbinfo = SHMEM_SB(sb);
>>
>> percpu_counter_destroy(&sbinfo->used_blocks);
>> + mpol_put(sbinfo->mpol);
>> kfree(sbinfo);
>> sb->s_fs_info = NULL;
>> }
>> --
>> 1.8.1.3
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] tmpfs: fix mempolicy object leaks
2013-03-03 23:49 ` Will Huck
@ 2013-03-05 19:40 ` Hugh Dickins
2013-03-07 6:41 ` Will Huck
0 siblings, 1 reply; 8+ messages in thread
From: Hugh Dickins @ 2013-03-05 19:40 UTC (permalink / raw)
To: Will Huck; +Cc: Greg Thelen, akpm, linux-mm, linux-kernel
On Mon, 4 Mar 2013, Will Huck wrote:
>
> Could you explain me why shmem has more relationship with mempolicy? It seems
> that there are many codes in shmem handle mempolicy, but other components in
> mm subsystem just have little.
NUMA mempolicy is mostly handled in mm/mempolicy.c, which services the
mbind, migrate_pages, set_mempolicy, get_mempolicy system calls: which
govern how process memory is distributed across NUMA nodes.
mm/shmem.c is affected because it was also found useful to specify
mempolicy on the shared memory objects which may back process memory:
that includes SysV SHM and POSIX shared memory and tmpfs. mm/hugetlb.c
contains some mempolicy handling for hugetlbfs; fs/ramfs is kept minimal,
so nothing in there.
Those are the memory-based filesystems, where NUMA mempolicy is most
natural. The regular filesystems could support shared mempolicy too,
but that would raise more awkward design questions.
Hugh
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] tmpfs: fix mempolicy object leaks
2013-03-05 19:40 ` Hugh Dickins
@ 2013-03-07 6:41 ` Will Huck
0 siblings, 0 replies; 8+ messages in thread
From: Will Huck @ 2013-03-07 6:41 UTC (permalink / raw)
To: Hugh Dickins; +Cc: Greg Thelen, akpm, linux-mm, linux-kernel
Hi Hugh,
On 03/06/2013 03:40 AM, Hugh Dickins wrote:
> On Mon, 4 Mar 2013, Will Huck wrote:
>> Could you explain me why shmem has more relationship with mempolicy? It seems
>> that there are many codes in shmem handle mempolicy, but other components in
>> mm subsystem just have little.
> NUMA mempolicy is mostly handled in mm/mempolicy.c, which services the
> mbind, migrate_pages, set_mempolicy, get_mempolicy system calls: which
> govern how process memory is distributed across NUMA nodes.
>
> mm/shmem.c is affected because it was also found useful to specify
> mempolicy on the shared memory objects which may back process memory:
> that includes SysV SHM and POSIX shared memory and tmpfs. mm/hugetlb.c
> contains some mempolicy handling for hugetlbfs; fs/ramfs is kept minimal,
> so nothing in there.
>
> Those are the memory-based filesystems, where NUMA mempolicy is most
> natural. The regular filesystems could support shared mempolicy too,
> but that would raise more awkward design questions.
I found that if mbind several processes to one node and almost exhaust
memory, processes will just stuck and no processes make progress or be
killed. Is it normal?
>
> Hugh
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-03-07 6:41 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-20 7:11 [PATCH 1/2] tmpfs: fix use-after-free of mempolicy object Greg Thelen
2013-02-20 7:11 ` [PATCH 2/2] tmpfs: fix mempolicy object leaks Greg Thelen
2013-02-20 20:26 ` Hugh Dickins
2013-02-20 21:06 ` Andrew Morton
2013-03-03 23:49 ` Will Huck
2013-03-05 19:40 ` Hugh Dickins
2013-03-07 6:41 ` Will Huck
2013-02-20 20:21 ` [PATCH 1/2] tmpfs: fix use-after-free of mempolicy object Hugh Dickins
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).