public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [Fwd: Re: [PATCH]:RESEND : Return proper error value on failure of dentry_open]
@ 2009-06-01  9:46 vibi sreenivasan
  2009-06-01  9:59 ` Jiri Slaby
  0 siblings, 1 reply; 6+ messages in thread
From: vibi sreenivasan @ 2009-06-01  9:46 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: Mimi Zohar, Kylene Hall, Serge Hallyn, Reiner Sailer,
	linux-kernel


hi,

> > >  	mutex_unlock(&iint->mutex);
> > > -	if (file)
> > > +	if (!IS_ERR(file))
> > >  		fput(file);
> > 
> > No, IS_ERR won't catch NULL and there is 'file = NULL' on the
> > dentry_open fail path.
Can you please point out that line number in fs/open.c
> 
> > I still think 'if (file)' is proper condition.
> > 
if(file) will be true for file == -ve error value
so in fput(file),
	atomic_long_dec_and_test(&file->f_count)
triggers bug
> 
> > What bug did you hit?
> > 
BUG: unable to handle kernel paging request at ffffffc9
IP: [<c048092e>] fput+0x2/0x14
*pde = 007ed067 *pte = 00000000 
Oops: 0002 [#1] PREEMPT 
last sysfs file: /sys/class/drm/card0/dev
Modules linked in: i915 drm i2c_algo_bit video output autofs4 af_packet
nf_conntrack_ipv4 nf_defrag_ipv4 ipt_REJECT iptable_filter ip_tables
nf_conntrack_netbios_ns xt_tcpudp nf_conntrack_ipv6 xt_state
nf_conntrack ip6t_ipv6header ip6t_REJECT ip6table_filter ip6_tables
x_tables ipv6 binfmt_misc dm_mirror dm_region_hash dm_log dm_multipath
dm_mod snd_intel8x0 snd_ac97_codec ac97_bus snd_seq_dummy snd_seq_oss
snd_seq_midi_event snd_seq snd_seq_device snd_pcm_oss snd_mixer_oss
snd_pcm fan sis900 mii snd_timer parport_pc parport rtc_cmos snd
rtc_core psmouse rtc_lib thermal soundcore floppy i2c_i801 button
thermal_sys serio_raw snd_page_alloc rng_core i2c_core pcspkr evdev unix
ata_piix libata sd_mod scsi_mod uhci_hcd ohci_hcd ehci_hcd [last
unloaded: processor]

Pid: 2213, comm: file Not tainted (2.6.30-rc7 #5)  
EIP: 0060:[<c048092e>] EFLAGS: 00210282 CPU: 0
EIP is at fput+0x2/0x14
EAX: ffffffb5 EBX: c5445a58 ECX: ffffffff EDX: ffffffb5
ESI: ca2b0ab0 EDI: ffffffb5 EBP: c070c1f9 ESP: ca2adeb8
 DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068
Process file (pid: 2213, ti=ca2ad000 task=ca05b480 task.ti=ca2ad000)
Stack:
 c05051d2 00000024 c5448780 00000000 00008001 00000004 c04876fd ca2adf0c
 c5445a00 ca2adf0c 00000000 ca2adf0c 00008001 c04898dd 00008000 debc8000
 00000017 000081a4 00000000 00000000 00000000 ca0d9680 c5445a00 00000000
Call Trace:
 [<c05051d2>] ? ima_path_check+0x14b/0x16c
 [<c04876fd>] ? may_open+0xb0/0x271
 [<c04898dd>] ? do_filp_open+0x3df/0x73d
 [<c0487aa7>] ? getname+0x1a/0xac
 [<c0490fe2>] ? alloc_fd+0xb8/0xd3
 [<c047e063>] ? do_sys_open+0x44/0xb4
 [<c047e117>] ? sys_open+0x1e/0x23
 [<c0402830>] ? sysenter_do_call+0x12/0x22
Code: 74 24 14 89 f9 89 ea 56 e8 38 fe ff ff 59 5e 89 d8 5b 5e 5f 5d c3
53 8b 1d 08 84 85 c0 89 1d a0 5e 76 c0 5b e9 0d 2b fa ff 89 c2 <ff> 48
14 0f 94 c0 84 c0 74 07 89 d0 e9 6d fc ff ff c3 56 89 c6 
EIP: [<c048092e>] fput+0x2/0x14 SS:ESP 0068:ca2adeb8
CR2: 00000000ffffffc9
---[ end trace 301fff67fb26bbae ]---

Thanks & Regards

vibi sreenivasan



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Fwd: Re: [PATCH]:RESEND : Return proper error value on failure of dentry_open]
  2009-06-01  9:46 [Fwd: Re: [PATCH]:RESEND : Return proper error value on failure of dentry_open] vibi sreenivasan
@ 2009-06-01  9:59 ` Jiri Slaby
  2009-06-01 10:02   ` Jiri Slaby
  0 siblings, 1 reply; 6+ messages in thread
From: Jiri Slaby @ 2009-06-01  9:59 UTC (permalink / raw)
  To: vibi_sreenivasan
  Cc: Mimi Zohar, Kylene Hall, Serge Hallyn, Reiner Sailer,
	linux-kernel, Eric Paris

On 06/01/2009 11:46 AM, vibi sreenivasan wrote:
>>>>  	mutex_unlock(&iint->mutex);
>>>> -	if (file)
>>>> +	if (!IS_ERR(file))
>>>>  		fput(file);
>>>
>>> No, IS_ERR won't catch NULL and there is 'file = NULL' on the
>>> dentry_open fail path.
> Can you please point out that line number in fs/open.c

Aha, you use linus's git. This has been fixed by
f06dd16a03f6f7f72fab4db03be36e28c28c6fd6
(IMA: Handle dentry_open failures)
in -next.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Fwd: Re: [PATCH]:RESEND : Return proper error value on failure of dentry_open]
  2009-06-01  9:59 ` Jiri Slaby
@ 2009-06-01 10:02   ` Jiri Slaby
  2009-06-01 10:37     ` vibi sreenivasan
  2009-06-01 14:45     ` Mimi Zohar
  0 siblings, 2 replies; 6+ messages in thread
From: Jiri Slaby @ 2009-06-01 10:02 UTC (permalink / raw)
  To: vibi_sreenivasan
  Cc: Mimi Zohar, Kylene Hall, Serge Hallyn, Reiner Sailer,
	linux-kernel, Eric Paris

On 06/01/2009 11:59 AM, Jiri Slaby wrote:
> On 06/01/2009 11:46 AM, vibi sreenivasan wrote:
>>>>>  	mutex_unlock(&iint->mutex);
>>>>> -	if (file)
>>>>> +	if (!IS_ERR(file))
>>>>>  		fput(file);
>>>>
>>>> No, IS_ERR won't catch NULL and there is 'file = NULL' on the
>>>> dentry_open fail path.
>> Can you please point out that line number in fs/open.c
> 
> Aha, you use linus's git. This has been fixed by
> f06dd16a03f6f7f72fab4db03be36e28c28c6fd6
> (IMA: Handle dentry_open failures)
> in -next.

[But the fix in -next still misses 'return 0' -> 'return rc' which I
though you had tried to change.]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Fwd: Re: [PATCH]:RESEND : Return proper error value on failure of dentry_open]
  2009-06-01 10:02   ` Jiri Slaby
@ 2009-06-01 10:37     ` vibi sreenivasan
  2009-06-01 14:45     ` Mimi Zohar
  1 sibling, 0 replies; 6+ messages in thread
From: vibi sreenivasan @ 2009-06-01 10:37 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: linux-kernel

hi,

> > Aha, you use linus's git. This has been fixed by
> > f06dd16a03f6f7f72fab4db03be36e28c28c6fd6
> > (IMA: Handle dentry_open failures)
> > in -next.
 Thanks alot .i was not much aware of linux-next usage.
 I will checkout that.
> 
> [But the fix in -next still misses 'return 0' -> 'return rc' which I
> though you had tried to change.]
 I will checkout as you said & try to put 'return rc' in place.

Thanks & regards
vibi sreenivasan
> 
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Fwd: Re: [PATCH]:RESEND : Return proper error value on failure of dentry_open]
  2009-06-01 10:02   ` Jiri Slaby
  2009-06-01 10:37     ` vibi sreenivasan
@ 2009-06-01 14:45     ` Mimi Zohar
  2009-06-02  5:44       ` vibi sreenivasan
  1 sibling, 1 reply; 6+ messages in thread
From: Mimi Zohar @ 2009-06-01 14:45 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: Eric Paris, Kylene J Hall, linux-kernel, Reiner Sailer, serue,
	vibi_sreenivasan, safford

Jiri Slaby <jirislaby@gmail.com> wrote on 06/01/2009 06:02:22 AM:

> Re: [Fwd: Re: [PATCH]:RESEND : Return proper error value on failure of 
dentry_open]
> 
> On 06/01/2009 11:59 AM, Jiri Slaby wrote:
> > On 06/01/2009 11:46 AM, vibi sreenivasan wrote:
> >>>>>     mutex_unlock(&iint->mutex);
> >>>>> -   if (file)
> >>>>> +   if (!IS_ERR(file))
> >>>>>        fput(file);
> >>>>
> >>>> No, IS_ERR won't catch NULL and there is 'file = NULL' on the
> >>>> dentry_open fail path.
> >> Can you please point out that line number in fs/open.c
> > 
> > Aha, you use linus's git. This has been fixed by
> > f06dd16a03f6f7f72fab4db03be36e28c28c6fd6
> > (IMA: Handle dentry_open failures)
> > in -next.
> 
> [But the fix in -next still misses 'return 0' -> 'return rc' which I
> though you had tried to change.]

At this point, we're just collecting measurements, not appraising 
them. The return code will be the result of appraise_measurement(). 
For now, the dentry_open shouldn't fail, but if it does, we probably
should audit it.

Mimi Zohar

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Fwd: Re: [PATCH]:RESEND : Return proper error value on failure of dentry_open]
  2009-06-01 14:45     ` Mimi Zohar
@ 2009-06-02  5:44       ` vibi sreenivasan
  0 siblings, 0 replies; 6+ messages in thread
From: vibi sreenivasan @ 2009-06-02  5:44 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Jiri Slaby, Eric Paris, Kylene J Hall, linux-kernel,
	Reiner Sailer, serue, safford


hi,
	Thanks for replying.
> > [But the fix in -next still misses 'return 0' -> 'return rc' which I
> > though you had tried to change.]
> 
> At this point, we're just collecting measurements, not appraising 
> them. The return code will be the result of appraise_measurement(). 
> For now, the dentry_open shouldn't fail, but if it does, we probably
> should audit it.
> 
Can you please change that comment.
Thanks & Regards
vibi sreenivasan
> Mimi Zohar
> 
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2009-06-02  5:42 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-06-01  9:46 [Fwd: Re: [PATCH]:RESEND : Return proper error value on failure of dentry_open] vibi sreenivasan
2009-06-01  9:59 ` Jiri Slaby
2009-06-01 10:02   ` Jiri Slaby
2009-06-01 10:37     ` vibi sreenivasan
2009-06-01 14:45     ` Mimi Zohar
2009-06-02  5:44       ` vibi sreenivasan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox