2004-04-01 19:11:43

by Sridhar Samudrala

[permalink] [raw]
Subject: CONFIG_DEBUG_PAGEALLOC and virt_addr_valid()

When CONFIG_DEBUG_PAGEALLOC is enabled, i am noticing that virt_addr_valid()
(called from sctp_is_valid_kaddr()) is returning true even for freed objects.
Is this a bug or expected behavior?

Thanks
Sridhar


2004-04-01 19:33:43

by Dave Hansen

[permalink] [raw]
Subject: Re: CONFIG_DEBUG_PAGEALLOC and virt_addr_valid()

On Thu, 2004-04-01 at 11:11, Sridhar Samudrala wrote:
> When CONFIG_DEBUG_PAGEALLOC is enabled, i am noticing that virt_addr_valid()
> (called from sctp_is_valid_kaddr()) is returning true even for freed objects.
> Is this a bug or expected behavior?

It's expected. Right now it just makes sure the address translates to a
valid pfn. Figuring out whether there are actually pagetables
underneath that address would require a pagetable walk.

Don't we unmap things when they are free'd with CONFIG_DEBUG_PAGEALLOC?
I guess we could add a pagetable walk in the debug case to
virt_addr_valid(), but it would probably make CONFIG_DEBUG_PAGEALLOC
even more of a dog than it already is.

-- Dave

2004-04-01 19:44:11

by Christoph Hellwig

[permalink] [raw]
Subject: Re: CONFIG_DEBUG_PAGEALLOC and virt_addr_valid()

On Thu, Apr 01, 2004 at 11:11:39AM -0800, Sridhar Samudrala wrote:
> When CONFIG_DEBUG_PAGEALLOC is enabled, i am noticing that virt_addr_valid()
> (called from sctp_is_valid_kaddr()) is returning true even for freed objects.
> Is this a bug or expected behavior?

Generally every use of virt_addr_valid() is a bug. What are you trying to
do?

2004-04-01 20:59:04

by Sridhar Samudrala

[permalink] [raw]
Subject: Re: CONFIG_DEBUG_PAGEALLOC and virt_addr_valid()

On Thu, 1 Apr 2004, Christoph Hellwig wrote:

> On Thu, Apr 01, 2004 at 11:11:39AM -0800, Sridhar Samudrala wrote:
> > When CONFIG_DEBUG_PAGEALLOC is enabled, i am noticing that virt_addr_valid()
> > (called from sctp_is_valid_kaddr()) is returning true even for freed objects.
> > Is this a bug or expected behavior?
>
> Generally every use of virt_addr_valid() is a bug. What are you trying to
> do?

We are trying to validate a kernel address that is passed by the user. Is
there a better way to do that?

When an SCTP association is established, the pointer to the association
structure is passed to the user as an identifier of the association. This
identifier is used in the later calls by the user.

-Sridhar

2004-04-01 21:24:53

by Richard B. Johnson

[permalink] [raw]
Subject: Re: CONFIG_DEBUG_PAGEALLOC and virt_addr_valid()

On Thu, 1 Apr 2004, Sridhar Samudrala wrote:

> On Thu, 1 Apr 2004, Christoph Hellwig wrote:
>
> > On Thu, Apr 01, 2004 at 11:11:39AM -0800, Sridhar Samudrala wrote:
> > > When CONFIG_DEBUG_PAGEALLOC is enabled, i am noticing that virt_addr_valid()
> > > (called from sctp_is_valid_kaddr()) is returning true even for freed objects.
> > > Is this a bug or expected behavior?
> >
> > Generally every use of virt_addr_valid() is a bug. What are you trying to
> > do?
>
> We are trying to validate a kernel address that is passed by the user. Is
> there a better way to do that?
>
> When an SCTP association is established, the pointer to the association
> structure is passed to the user as an identifier of the association. This
> identifier is used in the later calls by the user.
>
> -Sridhar

Are you now moving the protocol out of user-space and into the
kernel? If so, you should use the Unix/Linux methods of reading/
writing user memory.

Any protection is on a per-page basis. If you give a user some
pointer into the kernel, he can destroy a whole page before it
is detected! So, even if virt_addr_valid() did the "right" thing
you'd be in a lot of trouble using it.


Cheers,
Dick Johnson
Penguin : Linux version 2.4.24 on an i686 machine (797.90 BogoMips).
Note 96.31% of all statistics are fiction.


2004-04-02 00:27:39

by Andrew Morton

[permalink] [raw]
Subject: Re: CONFIG_DEBUG_PAGEALLOC and virt_addr_valid()

Sridhar Samudrala <[email protected]> wrote:
>
> On Thu, 1 Apr 2004, Christoph Hellwig wrote:
>
> > On Thu, Apr 01, 2004 at 11:11:39AM -0800, Sridhar Samudrala wrote:
> > > When CONFIG_DEBUG_PAGEALLOC is enabled, i am noticing that virt_addr_valid()
> > > (called from sctp_is_valid_kaddr()) is returning true even for freed objects.
> > > Is this a bug or expected behavior?
> >
> > Generally every use of virt_addr_valid() is a bug. What are you trying to
> > do?
>
> We are trying to validate a kernel address that is passed by the user. Is
> there a better way to do that?

yup. Pass the user an integer.

> When an SCTP association is established, the pointer to the association
> structure is passed to the user as an identifier of the association. This
> identifier is used in the later calls by the user.

Please don't do that. See lib/idr.c. I expect it does exactly what you
want.

2004-04-02 01:40:15

by Sridhar Samudrala

[permalink] [raw]
Subject: Re: CONFIG_DEBUG_PAGEALLOC and virt_addr_valid()

On Thu, 1 Apr 2004, Andrew Morton wrote:

> Sridhar Samudrala <[email protected]> wrote:
> >
> > On Thu, 1 Apr 2004, Christoph Hellwig wrote:
> >
> > > On Thu, Apr 01, 2004 at 11:11:39AM -0800, Sridhar Samudrala wrote:
> > > > When CONFIG_DEBUG_PAGEALLOC is enabled, i am noticing that virt_addr_valid()
> > > > (called from sctp_is_valid_kaddr()) is returning true even for freed objects.
> > > > Is this a bug or expected behavior?
> > >
> > > Generally every use of virt_addr_valid() is a bug. What are you trying to
> > > do?
> >
> > We are trying to validate a kernel address that is passed by the user. Is
> > there a better way to do that?
>
> yup. Pass the user an integer.
>
> > When an SCTP association is established, the pointer to the association
> > structure is passed to the user as an identifier of the association. This
> > identifier is used in the later calls by the user.
>
> Please don't do that. See lib/idr.c. I expect it does exactly what you
> want.

Yes. I think i should be able to use it to generate ids for the associations.
Thanks for pointing it out.
-Sridhar