[patch] allow write() on SOCK_PACKET sockets

[patch] allow write() on SOCK_PACKET sockets

[Stas Sergeev]

[-- Attachment #1: Type: text/plain, Size: 428 bytes --]

Hello.

Unless I am really missing something, there
seem to be no reason why the SOCK_PACKET code
does not allow to use write() or send() when
the socket was bound, and insists on using
sendto(). SOCK_RAW code, in comparison, allows
write() after bind().
Attached patch allows write() for the SOCK_PACKET
sockets when they are bound.
Can this be applied, or am I missing the point?

Signed-off-by: Stas Sergeev <stsp@aknet.ru>


[-- Attachment #2: pkt_send.diff --]
[-- Type: text/x-patch, Size: 916 bytes --]

--- linux/net/packet/af_packet.c	2004-06-26 15:20:54.000000000 +0400
+++ linux/net/packet/af_packet.c	2004-10-10 02:25:41.000000000 +0400
@@ -322,16 +322,23 @@
 			return(-EINVAL);
 		if (msg->msg_namelen==sizeof(struct sockaddr_pkt))
 			proto=saddr->spkt_protocol;
+		/*
+		 *	Find the device first to size check it 
+		 */
+
+		saddr->spkt_device[13] = 0;
+		dev = dev_get_by_name(saddr->spkt_device);
 	}
 	else
-		return(-ENOTCONN);	/* SOCK_PACKET must be sent giving an address */
-
-	/*
-	 *	Find the device first to size check it 
-	 */
+	{
+		struct packet_opt *po = pkt_sk(sk);
+		if (!po->running || !po->prot_hook.dev)
+			return -ENOTCONN;	/* SOCK_PACKET must be sent giving an address */
+		proto = po->prot_hook.type;
+		dev = po->prot_hook.dev;
+		dev_hold(dev);
+	}
 
-	saddr->spkt_device[13] = 0;
-	dev = dev_get_by_name(saddr->spkt_device);
 	err = -ENODEV;
 	if (dev == NULL)
 		goto out_unlock;

Re: [patch] allow write() on SOCK_PACKET sockets

[Herbert Xu]

Stas Sergeev <stsp@aknet.ru> wrote:
> 
> Unless I am really missing something, there
> seem to be no reason why the SOCK_PACKET code
> does not allow to use write() or send() when
> the socket was bound, and insists on using
> sendto(). SOCK_RAW code, in comparison, allows
> write() after bind().

It is counter-intuitive to allow write after bind().  AFAIK RAW
only allows write after connect(), not bind().

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

Re: [patch] allow write() on SOCK_PACKET sockets

[Stas Sergeev]

Hi Herbert.

Herbert Xu wrote:
>> sendto(). SOCK_RAW code, in comparison, allows
>> write() after bind().
> It is counter-intuitive to allow write after bind().  AFAIK RAW
> only allows write after connect(), not bind().
I claim that SOCK_RAW allows write() after bind()
because a few days ago I changed dosemu code
to use SOCK_RAW instead of SOCK_PACKET and write()
instead of sendto(). See here:
http://cvs.sourceforge.net/viewcvs.py/dosemu/dosemu/src/dosext/net/net/libpacket.c?r1=1.4&r2=1.5&diff_format=u
http://cvs.sourceforge.net/viewcvs.py/dosemu/dosemu/src/dosext/net/net/pktnew.c?r1=1.7&r2=1.8&diff_format=u
Of course the fact that I did that for dosemu,
doesn't mean that I was doing the right thing
(so if you know it is wrong - I'll redo it)
but at least it was tested and works.
And since that works for SOCK_RAW, I don't
see any reasons for it to not work for
SOCK_PACKET. And btw, I can use read() quite
happily even with SOCK_PACKET, so why not
write()...
My patch is simply an adoption of the code
SOCK_RAW has. See af_packet.c:packet_sendmsg(),
you'll see under "if (saddr == NULL)" just the
same code as I was doing for SOCK_PACKET.

Re: [patch] allow write() on SOCK_PACKET sockets

[Herbert Xu]

Stas Sergeev <stsp@aknet.ru> wrote:
>
> I claim that SOCK_RAW allows write() after bind()
> because a few days ago I changed dosemu code
> to use SOCK_RAW instead of SOCK_PACKET and write()

Well I just checked net/ipv4/raw.c and it's pretty clear that it does

		err = -EDESTADDRREQ;
		if (sk->sk_state != TCP_ESTABLISHED)
			goto out;

So you need to connect before you can write.  I'm intrigued that
you can write before connecting on a raw socket.  Could you please
write up a minimal program that I can play with?

> SOCK_PACKET. And btw, I can use read() quite
> happily even with SOCK_PACKET, so why not
> write()...

Well read() is different.  It returns all packets received on
the socket, regardless of where they came from.  So for a
connectionless socket it makes perfect sense to read() before
you've called connect().

OTOH, write() and send() needs to know where the message is going
to.  That's exactly what connect() provides.  So it makes no sense
to write()/send() before you've called connect().

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

Re: [patch] allow write() on SOCK_PACKET sockets

[Stas Sergeev]

Hi.

Herbert Xu wrote:
Stas Sergeev <stsp@aknet.ru> wrote:
>> I claim that SOCK_RAW allows write() after bind()
>> because a few days ago I changed dosemu code
>> to use SOCK_RAW instead of SOCK_PACKET and write()
> Well I just checked net/ipv4/raw.c and it's pretty clear that it does
I think you are looking at a wrong place.
You are looking into IP raw sockets code.
Packet sockets are really the different
layer. Please have a look into
net/packet/af_packet.c instead.

> So you need to connect before you can write.
Packet sockets, actually, do not even have
connect:
.connect = sock_no_connect

> I'm intrigued that
> you can write before connecting on a raw socket.
Not an IP raw socket, but the raw packet
socket. So yes, I can. And that looks very
natural to me, not a hack or something.

> Could you please
> write up a minimal program that I can play with?
I can but I am a bit surprised that dosemu
is not a sufficient test-case for *you* :)
But I don't seem to be able to send any
mail to you:

<herbert@gondor.apana.org.au>: host arnor.apana.org.au[203.14.152.115] said:
    550 mail from 217.67.122.194 rejected: administrative prohibition (in reply
    to RCPT TO command)

> Well read() is different.
Yes, not a good argument on my side, sorry.

> OTOH, write() and send() needs to know where the message is going
> to.
That's exactly where the packet sockets are
different. Here's the whole point. Have a
look into a "struct sockaddr_pkt":

              struct sockaddr_pkt
              {
                  unsigned short  spkt_family;
                  unsigned char   spkt_device[14];
                  unsigned short  spkt_protocol;
              };

Not too much about a destination here.
For the packet sockets you only need to
know via which eth device you want to send
it, and nothing more. And this is what I
specify to bind() anyway, so I dont want
to duplicate that info all the time.
You say it is counter-intuitive.
I'll agree with this only if you point me
another convinient way to bind to the
particular eth device and send/receive via
that device without always specifying its
name/number. That's what SOCK_RAW allows,
but not SOCK_PACKET.

My patch is probably dead anyway though.
SOCK_PACKET is mentioned to be deprecated
in man, so perhaps noone will apply any
patches on it... Just wanted to point out
that there is a bug/inconsistency in it.

Re: [patch] allow write() on SOCK_PACKET sockets

[Herbert Xu]

Stas Sergeev <stsp@aknet.ru> wrote:
>
> I think you are looking at a wrong place.
> You are looking into IP raw sockets code.
> Packet sockets are really the different
> layer. Please have a look into
> net/packet/af_packet.c instead.

Yes.  Sorry for the confusion.

> But I don't seem to be able to send any
> mail to you:

Should work now.
 
>> OTOH, write() and send() needs to know where the message is going
>> to.
>
> That's exactly where the packet sockets are
> different. Here's the whole point. Have a
> look into a "struct sockaddr_pkt":
> 
>              struct sockaddr_pkt
>              {
>                  unsigned short  spkt_family;
>                  unsigned char   spkt_device[14];
>                  unsigned short  spkt_protocol;
>              };

I see your point.  But I don't really like the current code that
uses the address from bind for sending.  Even though it works here
because the packet socket is symmetric wrt sending/receiving, it
is counter-intuitive for the socket API in general.

> My patch is probably dead anyway though.
> SOCK_PACKET is mentioned to be deprecated
> in man, so perhaps noone will apply any
> patches on it... Just wanted to point out

Indeed it is.

> that there is a bug/inconsistency in it.

Thanks anyway.

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