Date: Thu, 8 Apr 2004 20:38:55 +0300 From: Ruslan Ermilov <ru@FreeBSD.org> To: Archie Cobbs <archie@FreeBSD.org>, Julian Elischer <julian@FreeBSD.org> Cc: net@FreeBSD.org Subject: Bugs in ASCII2BINARY handling Message-ID: <20040408173855.GB1579@ip.net.ua>
next in thread | raw e-mail | index | archive | help
--kXdP64Ggrk/fb43R
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
Hi,
I've been experimenting with IP multicasting under Netgraph, and
needed to use setsockopt(2)/getsockopt(2) on an ng_ksocket(4) node
using the ASCII form of the "setopt" and "getopt" messages.
Problem #1: ASCII message "getopt" doesn't work.
: + mkpeer . ksocket s inet/dgram/udp
: + msg .:s getopt { level=3D0 name=3D10 }
: ngctl: send msg: Invalid argument
Analysis:
The NGM_KSOCKET_GETOPT message type handler expects the argument
of exactly sizeof(struct ng_ksocket_sockopt) =3D=3D 8 bytes long.
When doing the ASCII2BINARY conversion (in ng_base.c), the "ascii"
message's header gets copied into the "binary" message header,
including the header.arglen, which is 20 in the above example,
strlen("{ level=3D0 name=3D10 }") + 1.
Since "struct ng_ksocket_sockopt" doesn't include the field to
denote the length of the "value" byte array, the function
ng_parse_sockoptval_getLength() computes the length of the "value"
byte array (which is not present in the above "getopt" message
at all) using the passed message's argument len, which in the
above example equals 20 - sizeof(struct ng_ksocket_sockopt) =3D=3D 12,
instead of the expected 0, hence the handler returns EINVAL.
Workaround to make "getopt" message work:
%%%
Index: ng_ksocket.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /home/ncvs/src/sys/netgraph/ng_ksocket.c,v
retrieving revision 1.39
diff -u -p -r1.39 ng_ksocket.c
--- ng_ksocket.c 26 Jan 2004 14:05:31 -0000 1.39
+++ ng_ksocket.c 8 Apr 2004 14:22:34 -0000
@@ -809,7 +809,7 @@ ng_ksocket_rcvmsg(node_p node, item_p it
struct sockopt sopt;
=20
/* Sanity check */
- if (msg->header.arglen !=3D sizeof(*ksopt))
+ if (msg->header.arglen < sizeof(*ksopt))
ERROUT(EINVAL);
if (so =3D=3D NULL)
ERROUT(ENXIO);
%%%
Problem #2: ASCII message "setopt" may not always work.
Analysis:
Basically we have the same problem, because the "binary" message
gets arglen from its "ascii" representation. The result is that
we either send the excessive bytes:
: + mkpeer . ksocket s inet/dgram/udp
: + debug 2
: + msg .:s setopt { level=3D0 name=3D10 value=3D[ 3 ] }
: [...]
: ngctl: SENDING MESSAGE:
: ngctl: SOCKADDR: { fam=3D32 len=3D6 addr=3D".:s" }
: ngctl: NG_MESG :
: ngctl: vers 0
: ngctl: arglen 32
: ngctl: flags 0
: ngctl: token 19
: ngctl: cookie KSOCKET (942710669)
: ngctl: cmd 7
: ngctl: args (32 bytes)
: ngctl: 0000: 00 00 00 00 0a 00 00 00 03 00 00 00 00 00 00 00 .........=
=2E......
: ngctl: 0010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 .........=
=2E......
(everything after byte 03 shouldn't have been sent), or it doesn't
work at all:
: + msg .:s setopt { level=3D0 name=3D10 value=3D[ 40=3D3 ] }
: ngctl: send msg: Argument list too long
In the first case, we send excessive bytes because the function
ng_parse_sockoptval_getLength() computes incorrect length of the
"value" byte array, which is arglen - 8, where arglen is actually
a rounded up strlen() of the ASCII representation of the message
(I'm clueless as to why it gets rounded to the 4 byte boundary).
In the second case, ASCII2BINARY bogusly assumes that the binary
form will be always shorter than its ASCII representation, so it
sets binary's arglen to that of ascii's arglen, albeit it uses
a static buffer of 2000 bytes. This should be easily fixed:
%%%
Index: ng_base.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /home/ncvs/src/sys/netgraph/ng_base.c,v
retrieving revision 1.74
diff -u -p -r1.74 ng_base.c
--- ng_base.c 26 Jan 2004 14:05:30 -0000 1.74
+++ ng_base.c 8 Apr 2004 14:00:58 -0000
@@ -2805,6 +2805,7 @@ ng_generic_msg(node_p here, item_p item,
=20
/* Copy ASCII message header to response message payload */
bcopy(ascii, binary, sizeof(*ascii));
+ binary->header.arglen =3D bufSize;
=20
/* Find command by matching ASCII command string */
for (c =3D here->nd_type->cmdlist;
%%%
Conclusion: ASCII2BINARY doesn't behave well with variable size
arrays that do not have a field describing their size. If
"getopt" and "setopt" had such a field, only second patch would
be necessary, which is generic enough anyway.
Archie, Julian, could you please comment on the patches proposed?
Cheers,
--=20
Ruslan Ermilov
ru@FreeBSD.org
FreeBSD committer
--kXdP64Ggrk/fb43R
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (FreeBSD)
iD8DBQFAdY4vUkv4P6juNwoRAn7vAJ4mZJ/BKE9KDW2Iv9EGeN27cYkoxQCfUdJI
+eVgY2uQ0MjDJkIT1DA36Rk=
=Yaj1
-----END PGP SIGNATURE-----
--kXdP64Ggrk/fb43R--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040408173855.GB1579>