On Mon, Nov 22, 2010 at 10:09 -0800, Andrew Morton wrote:
> Arguably we should just let the allocation attempt pass through to
> kmalloc() and let kmalloc() return an error if it was too large. Possibly
> we need a __GFP_NOWARN in there somewhere. Please send us that stack
> dump?
void *p = kmalloc(5L*1024*1024, GFP_KERNEL);
pr_err("p = %p\n", p);
[13896.001970] ------------[ cut here ]------------
[13896.001979] WARNING: at
/build/buildd/linux-lts-backport-maverick-2.6.35/mm/page_alloc.c:1968
__alloc_pages_slowpath+0x44b/0x590()
[13896.001983] Hardware name: Q310
[13896.001984] Modules linked in: test(+) easy_slow_down_manager rfcomm
sco bridge stp bnep l2cap vboxnetadp vboxnetflt vboxdrv nfsd exportfs
nfs lockd fscache nfs_acl auth_rpcgss sunrpc samsung_backlight
binfmt_misc ppdev input_polldev lp parport snd_seq_dummy uvcvideo
videodev v4l1_compat v4l2_compat_ioctl32 iTCO_vendor_support dm_crypt
usb_storage aes_x86_64 aes_generic vfat fat nls_cp437 nls_iso8859_1
usblp joydev arc4 btusb bluetooth snd_hda_intel snd_hda_codec snd_hwdep
snd_pcm snd_seq_midi snd_rawmidi snd_seq_midi_event snd_seq snd_timer
snd_seq_device nouveau ath5k ttm mac80211 drm_kms_helper ath snd
cfg80211 video output drm soundcore led_class psmouse snd_page_alloc
sky2 serio_raw i2c_algo_bit intel_agp [last unloaded: test]
[13896.002045] Pid: 27627, comm: insmod Not tainted 2.6.35-22-generic
#34~lucid1-Ubuntu
[13896.002047] Call Trace:
[13896.002055] [<ffffffff8106079f>] warn_slowpath_common+0x7f/0xc0
[13896.002059] [<ffffffff810607fa>] warn_slowpath_null+0x1a/0x20
[13896.002063] [<ffffffff811081bb>] __alloc_pages_slowpath+0x44b/0x590
[13896.002067] [<ffffffff8112aaae>] ? lazy_max_pages+0x1e/0x30
[13896.002071] [<ffffffff81104f74>] ? __probe_kernel_write+0x44/0x70
[13896.002075] [<ffffffff8110849a>] __alloc_pages_nodemask+0x19a/0x1f0
[13896.002080] [<ffffffff8113a30a>] alloc_pages_current+0x9a/0x100
[13896.002084] [<ffffffffa01cb020>] ? hello_init+0x0/0xa0 [test]
[13896.002088] [<ffffffff8110736e>] __get_free_pages+0xe/0x50
[13896.002091] [<ffffffffa01cb04f>] hello_init+0x2f/0xa0 [test]
[13896.002095] [<ffffffffa01cb020>] ? hello_init+0x0/0xa0 [test]
[13896.002099] [<ffffffff8100204c>] do_one_initcall+0x3c/0x1a0
[13896.002104] [<ffffffff8109bceb>] sys_init_module+0xbb/0x200
[13896.002109] [<ffffffff8100a0f2>] system_call_fastpath+0x16/0x1b
[13896.002112] ---[ end trace b991655c592a1d6e ]---
[13896.002114] p = (null)
> And perhaps
> strndup_user() needs __GFP_NOWARN treatment.
strndup_user() silently return ERR_PTR(-EINVAL) if string is too long.
> The code should be using strndup_user() anyway.
Smth like this?
diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
index 09e3090..3afd249 100644
--- a/drivers/gpu/vga/vgaarb.c
+++ b/drivers/gpu/vga/vgaarb.c
@@ -836,19 +836,10 @@ static ssize_t vga_arb_write(struct file *file, const char __user * buf,
int ret_val;
int i;
- if (count > PAGE_SIZE)
- count = PAGE_SIZE;
-
- kbuf = kmalloc(count + 1, GFP_KERNEL);
- if (!kbuf)
- return -ENOMEM;
-
- if (copy_from_user(kbuf, buf, count)) {
- kfree(kbuf);
- return -EFAULT;
- }
+ kbuf = strndup_user(buf, PAGE_SIZE);
+ if (IS_ERR(kbuf))
+ return PTR_ERR(kbuf);
curr_pos = kbuf;
- kbuf[count] = '\0'; /* Just to make sure... */
if (strncmp(curr_pos, "lock ", 5) == 0) {
curr_pos += 5;
--
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
