udp.c

udp.c

[Rommer]

Hello,

Where used strange function udp_v4_hash?
linux-2.6.11.11, net/ipv4/udp.c:204

static void udp_v4_hash(struct sock *sk)
{
	BUG();
}

-- 
Best regards, Roman

Re: udp.c

[Bernd Petrovitsch]

On Mon, 2005-06-13 at 14:57 +0300, Rommer wrote:
> Where used strange function udp_v4_hash?
> linux-2.6.11.11, net/ipv4/udp.c:204
> 
> static void udp_v4_hash(struct sock *sk)

Since it is "static" the user must be in the same source file (or -
theoretically - any included header).
So just use the "find" function of your $PREFERRED_EDITOR.

	Bernd
-- 
Firmix Software GmbH                   http://www.firmix.at/
mobil: +43 664 4416156                 fax: +43 1 7890849-55
          Embedded Linux Development and Services

Re: udp.c

[Adrian Bunk]

On Mon, Jun 13, 2005 at 02:57:23PM +0300, Rommer wrote:

> Hello,

Hi Roman,

> Where used strange function udp_v4_hash?
> linux-2.6.11.11, net/ipv4/udp.c:204
>...

do a simple

  grep -r udp_v4_hash *

in the main directory of the kernel sources and you'll find it.

> Best regards, Roman

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: udp.c

[Måns Rullgård]

Bernd Petrovitsch <bernd@firmix.at> writes:

> On Mon, 2005-06-13 at 14:57 +0300, Rommer wrote:
>> Where used strange function udp_v4_hash?
>> linux-2.6.11.11, net/ipv4/udp.c:204
>> 
>> static void udp_v4_hash(struct sock *sk)
>
> Since it is "static" the user must be in the same source file (or -
> theoretically - any included header).

It's not that simple.  It is assigned to the 'hash' field of a struct
proto, which is exported.  It could be used from anywhere, but
hopefully isn't.  Something else is supposed to ensure that it is
never called when using the UDP protocol.

-- 
Måns Rullgård
mru@inprovide.com

Re: udp.c

[Bernd Petrovitsch]

On Mon, 2005-06-13 at 14:24 +0200, Måns Rullgård wrote:
> Bernd Petrovitsch <bernd@firmix.at> writes:
> 
> > On Mon, 2005-06-13 at 14:57 +0300, Rommer wrote:
> >> Where used strange function udp_v4_hash?
> >> linux-2.6.11.11, net/ipv4/udp.c:204
> >> 
> >> static void udp_v4_hash(struct sock *sk)
> >
> > Since it is "static" the user must be in the same source file (or -
> > theoretically - any included header).
> 
> It's not that simple.  It is assigned to the 'hash' field of a struct

If you interpret "called" word-by-word yes. I assumed "used".

> proto, which is exported.  It could be used from anywhere, but

The the OP has to grep for dereferences for this hash variable and check
if it is (or may be) from the given struct.
Well, that's the virtue of object-orientation: Follow the objects, not
the functions/methods.

> hopefully isn't.  Something else is supposed to ensure that it is
> never called when using the UDP protocol.

Apparently.

	Bernd
-- 
Firmix Software GmbH                   http://www.firmix.at/
mobil: +43 664 4416156                 fax: +43 1 7890849-55
          Embedded Linux Development and Services

Re: udp.c

[Rommer]

Hello,

So, why BUG(), not just void function?

Bernd Petrovitsch wrote:
> On Mon, 2005-06-13 at 14:24 +0200, Måns Rullgård wrote:
> 
>>Bernd Petrovitsch <bernd@firmix.at> writes:
>>
>>
>>>On Mon, 2005-06-13 at 14:57 +0300, Rommer wrote:
>>>
>>>>Where used strange function udp_v4_hash?
>>>>linux-2.6.11.11, net/ipv4/udp.c:204
>>>>
>>>>static void udp_v4_hash(struct sock *sk)
>>>
>>>Since it is "static" the user must be in the same source file (or -
>>>theoretically - any included header).
>>
>>It's not that simple.  It is assigned to the 'hash' field of a struct
> 
> 
> If you interpret "called" word-by-word yes. I assumed "used".
> 
> 
>>proto, which is exported.  It could be used from anywhere, but
> 
> 
> The the OP has to grep for dereferences for this hash variable and check
> if it is (or may be) from the given struct.
> Well, that's the virtue of object-orientation: Follow the objects, not
> the functions/methods.
> 
> 
>>hopefully isn't.  Something else is supposed to ensure that it is
>>never called when using the UDP protocol.
> 
> 
> Apparently.
> 
> 	Bernd

-- 
Best regards, Roman

Re: udp.c

[Bernd Petrovitsch]

On Mon, 2005-06-13 at 15:54 +0300, Rommer wrote:
> So, why BUG(), not just void function?

So that the kernel oopses and someone notices that serious breakage
immediately.

	Bernd
-- 
Firmix Software GmbH                   http://www.firmix.at/
mobil: +43 664 4416156                 fax: +43 1 7890849-55
          Embedded Linux Development and Services

Re: udp.c

[Måns Rullgård]

Rommer <rommer@active.by> writes:

> Bernd Petrovitsch wrote:
>> On Mon, 2005-06-13 at 14:24 +0200, Måns Rullgård wrote:
>>
>>>Bernd Petrovitsch <bernd@firmix.at> writes:
>>>
>>>
>>>>On Mon, 2005-06-13 at 14:57 +0300, Rommer wrote:
>>>>
>>>>>Where used strange function udp_v4_hash?
>>>>>linux-2.6.11.11, net/ipv4/udp.c:204
>>>>>
>>>>>static void udp_v4_hash(struct sock *sk)
>>>>
>>>>Since it is "static" the user must be in the same source file (or -
>>>>theoretically - any included header).
>>>
>>>It's not that simple.  It is assigned to the 'hash' field of a struct
>> If you interpret "called" word-by-word yes. I assumed "used".
>>
>>>proto, which is exported.  It could be used from anywhere, but
>> The the OP has to grep for dereferences for this hash variable and
>> check
>> if it is (or may be) from the given struct.
>> Well, that's the virtue of object-orientation: Follow the objects, not
>> the functions/methods.
>>
>>>hopefully isn't.  Something else is supposed to ensure that it is
>>>never called when using the UDP protocol.
>
> So, why BUG(), not just void function?

Calling the function would be the result of a bug elsewhere in the
code, which should be detected and reported.

-- 
Måns Rullgård
mru@inprovide.com

Re: udp.c

[Jesper Juhl]

On 6/13/05, Måns Rullgård <mru@inprovide.com> wrote:
> Rommer <rommer@active.by> writes:
> 
> > Bernd Petrovitsch wrote:
> >> On Mon, 2005-06-13 at 14:24 +0200, Måns Rullgård wrote:
> >>
> >>>Bernd Petrovitsch <bernd@firmix.at> writes:
> >>>
> >>>
> >>>>On Mon, 2005-06-13 at 14:57 +0300, Rommer wrote:
> >>>>
> >>>>>Where used strange function udp_v4_hash?
> >>>>>linux-2.6.11.11, net/ipv4/udp.c:204
> >>>>>
> >>>>>static void udp_v4_hash(struct sock *sk)
> >>>>
> >>>>Since it is "static" the user must be in the same source file (or -
> >>>>theoretically - any included header).
> >>>
> >>>It's not that simple.  It is assigned to the 'hash' field of a struct
> >> If you interpret "called" word-by-word yes. I assumed "used".
> >>
> >>>proto, which is exported.  It could be used from anywhere, but
> >> The the OP has to grep for dereferences for this hash variable and
> >> check
> >> if it is (or may be) from the given struct.
> >> Well, that's the virtue of object-orientation: Follow the objects, not
> >> the functions/methods.
> >>
> >>>hopefully isn't.  Something else is supposed to ensure that it is
> >>>never called when using the UDP protocol.
> >
> > So, why BUG(), not just void function?
> 
> Calling the function would be the result of a bug elsewhere in the
> code, which should be detected and reported.
> 
Why not remove the function and audit the code for users (and if any,
remove them)...? Let's get rid of it instead of having a function sit
around the only purpose of which is to BUG();

-- 
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html

Re: udp.c

[David S. Miller]

From: Jesper Juhl <jesper.juhl@gmail.com>
Date: Mon, 13 Jun 2005 19:23:36 +0200

> Why not remove the function and audit the code for users (and if any,
> remove them)...? Let's get rid of it instead of having a function sit
> around the only purpose of which is to BUG();

Then if someone breaks that invariant, we'll never find out.

The code is staying, sorry.

Re: udp.c

[Herbert Xu]

David S. Miller <davem@davemloft.net> wrote:
> From: Jesper Juhl <jesper.juhl@gmail.com>
> Date: Mon, 13 Jun 2005 19:23:36 +0200
> 
>> Why not remove the function and audit the code for users (and if any,
>> remove them)...? Let's get rid of it instead of having a function sit
>> around the only purpose of which is to BUG();
> 
> Then if someone breaks that invariant, we'll never find out.
> 
> The code is staying, sorry.

It'll dump the stack anyway if we just make it a NULL pointer.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -201,11 +201,6 @@ fail:
 	return 1;
 }
 
-static void udp_v4_hash(struct sock *sk)
-{
-	BUG();
-}
-
 static void udp_v4_unhash(struct sock *sk)
 {
 	write_lock_bh(&udp_hash_lock);
@@ -1370,7 +1365,7 @@ struct proto udp_prot = {
 	.recvmsg =	udp_recvmsg,
 	.sendpage =	udp_sendpage,
 	.backlog_rcv =	udp_queue_rcv_skb,
-	.hash =		udp_v4_hash,
+	/* .hash is intentionally left NULL */
 	.unhash =	udp_v4_unhash,
 	.get_port =	udp_v4_get_port,
 	.obj_size =	sizeof(struct udp_sock),
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -135,11 +135,6 @@ fail:
 	return 1;
 }
 
-static void udp_v6_hash(struct sock *sk)
-{
-	BUG();
-}
-
 static void udp_v6_unhash(struct sock *sk)
 {
  	write_lock_bh(&udp_hash_lock);
@@ -1048,7 +1043,7 @@ struct proto udpv6_prot = {
 	.sendmsg =	udpv6_sendmsg,
 	.recvmsg =	udpv6_recvmsg,
 	.backlog_rcv =	udpv6_queue_rcv_skb,
-	.hash =		udp_v6_hash,
+	/* .hash is intentionally left NULL */
 	.unhash =	udp_v6_unhash,
 	.get_port =	udp_v6_get_port,
 	.obj_size =	sizeof(struct udp6_sock),

Re: udp.c

[David S. Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Tue, 14 Jun 2005 07:42:52 +1000

> It'll dump the stack anyway if we just make it a NULL pointer.

Some platforms don't handle that very cleanly, for example
it may be necessary to have something mapped at page zero
for one reason or another.

Re: udp.c

[Herbert Xu]

On Mon, Jun 13, 2005 at 02:57:16PM -0700, David S. Miller wrote:
> From: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Tue, 14 Jun 2005 07:42:52 +1000
> 
> > It'll dump the stack anyway if we just make it a NULL pointer.
> 
> Some platforms don't handle that very cleanly, for example
> it may be necessary to have something mapped at page zero
> for one reason or another.

Are there any existing platforms that do that in kernel mode?
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: udp.c

[David S. Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Tue, 14 Jun 2005 09:04:22 +1000

> On Mon, Jun 13, 2005 at 02:57:16PM -0700, David S. Miller wrote:
> > From: Herbert Xu <herbert@gondor.apana.org.au>
> > Date: Tue, 14 Jun 2005 07:42:52 +1000
> > 
> > > It'll dump the stack anyway if we just make it a NULL pointer.
> > 
> > Some platforms don't handle that very cleanly, for example
> > it may be necessary to have something mapped at page zero
> > for one reason or another.
> 
> Are there any existing platforms that do that in kernel mode?

X86 did, especially during bootup, for a long time.

I know the highly optimized sparc64 instruction TLB miss handler
doesn't handle this properly and this usually hangs the machine.
I've put some checks in there that tries to handle it properly,
but there are still some cases that pass through.

Re: udp.c

[Andi Kleen]

"David S. Miller" <davem@davemloft.net> writes:

> From: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Tue, 14 Jun 2005 09:04:22 +1000
>
>> On Mon, Jun 13, 2005 at 02:57:16PM -0700, David S. Miller wrote:
>> > From: Herbert Xu <herbert@gondor.apana.org.au>
>> > Date: Tue, 14 Jun 2005 07:42:52 +1000
>> > 
>> > > It'll dump the stack anyway if we just make it a NULL pointer.
>> > 
>> > Some platforms don't handle that very cleanly, for example
>> > it may be necessary to have something mapped at page zero
>> > for one reason or another.
>> 
>> Are there any existing platforms that do that in kernel mode?
>
> X86 did, especially during bootup, for a long time.

Still does, as does x86-64, but actually it could be changed now using
change_page_attr (and then later undoing it). Is it worth it?

-Andi

Re: udp.c

[David S. Miller]

From: Andi Kleen <ak@muc.de>
Date: Tue, 14 Jun 2005 01:53:56 +0200

> Still does, as does x86-64, but actually it could be changed now using
> change_page_attr (and then later undoing it). Is it worth it?

A lot of bootup OOPS's have occured in the past and
were never discovered until someone on a non-x86
platform tested the patch.

Re: udp.c

[Andi Kleen]

On Mon, Jun 13, 2005 at 05:00:45PM -0700, David S. Miller wrote:
> From: Andi Kleen <ak@muc.de>
> Date: Tue, 14 Jun 2005 01:53:56 +0200
> 
> > Still does, as does x86-64, but actually it could be changed now using
> > change_page_attr (and then later undoing it). Is it worth it?
> 
> A lot of bootup OOPS's have occured in the past and
> were never discovered until someone on a non-x86
> platform tested the patch.

Ok I can fix it, but only reasonably after mem_init (at least without
hacking up change_page_attr a lot to deal with bootmem).
Is that still worth it? 

-Andi

Re: udp.c

[David S. Miller]

From: Andi Kleen <ak@muc.de>
Date: 14 Jun 2005 02:12:49 +0200,Tue, 14 Jun 2005 02:12:49 +0200

> Ok I can fix it, but only reasonably after mem_init (at least without
> hacking up change_page_attr a lot to deal with bootmem).
> Is that still worth it? 

I think so.