The sysctl kern.geom.confxml can return invalid XML if, for example, the label has characters that are not safe in XML. This breaks libgeom's geom_xml2tree and utilities that use it, like glabel and gstat. Fix: geom_dump.c should encode XML entities. Attached is a patch. Patch attached with submission follows: How-To-Repeat: # glabel list # mdconfig -at swap -s 1m md0 # glabel create bread\&butter /dev/md0 GEOM_LABEL: Label for provider md0 is label/bread&butter. # glabel list Cannot get GEOM tree: Unknown error: -1. # gstat gstat: geom_gettree = -1: Unknown error: 0
Responsible Changed From-To: freebsd-bugs->freebsd-geom Over to maintainer(s).
In message <200610132043.k9DKh3lW055022@www.freebsd.org>, douglas steinwand wri tes: >geom_dump.c should encode XML entities. Attached is a patch. I don't like it in two ways. I'm not keep on the encoding, and would be inclined to say "Don't use such names then", but I can probably be convinced that this is actually a good idea if sensible examples are shown. The other thing is that in the patch, the _encode_entities() function should not have a leading underscore and should take arguments: static void encode_entities(struct sbuf *sb, const char *fmt, const char *str, int len); So that usage would not need a randomsized local buffer and double enveloping of the call: >- sbuf_printf(sb, "\t <name>%s</name>\n", pp->name); Should be: encode_entities(sb, "\t <name>%s</name>\n", pp->name, -1); (-1 for len means "use strlen") -- Poul-Henning Kamp | UNIX since Zilog Zeus 3.20 phk@FreeBSD.ORG | TCP/IP since RFC 956 FreeBSD committer | BSD since 4.3-tahoe Never attribute to malice what can adequately be explained by incompetence.
* Poul-Henning Kamp <phk@phk.freebsd.dk> [2006-10-15 18:12:56 +0000]: > I'm not keep on the encoding, and would be inclined to say > "Don't use such names then", but I can probably be convinced > that this is actually a good idea if sensible examples are shown. I encountered this issue thanks to a CD from Sun Microsystems: # glabel status Name Status Components iso9660/tools_&_driver_1.0 N/A acd0 The label on its ISO9660 filesystem has "&", which must be encoded in XML. > The other thing is that in the patch, the _encode_entities() > function should not have a leading underscore and should > take arguments: > > static void > encode_entities(struct sbuf *sb, const char *fmt, > const char *str, int len); My initial patch was just a quick hack to work around the problem. I didn't know how FreeBSD's developers wanted to address it. For example, if XML will be used widely in the kernel, should a new format code for kvprintf() do this XML encoding? > So that usage would not need a randomsized local buffer and > double enveloping of the call: > > >- sbuf_printf(sb, "\t <name>%s</name>\n", pp->name); > > Should be: > > encode_entities(sb, "\t <name>%s</name>\n", pp->name, -1); > > (-1 for len means "use strlen") Attached is a new patch for geom_dump.c which follows your recommendations. Thanks, - doug
Hi, today I tried PJDs ZFS patches. After invoking zpool create I always got a coredump. After further debugging I recognized gstat isn't working either, which seems to be because of an NTFS partition with a certain name not correctly encoded as XML specification requires. Its the German "Lokaler Datenträger" which is the defaultname for drive "c". So we have to deal with those characters. I'm voting against encoding specification or something like that, but I do recommend using the escape method. For example using "&#e4;" for encoding E4 german ä and < for a "<" character. This way we can escape all illegal characters. What do you suggest? regards, -Dennis Berger
Dennis Berger schrieb: > Hi, > today I tried PJDs ZFS patches. After invoking zpool create I always got > a coredump. > After further debugging I recognized gstat isn't working either, which > seems to be because of an NTFS partition with a certain name not > correctly encoded as XML specification requires. Its the German "Lokaler > Datenträger" which is the defaultname for drive "c". So we have to deal > with those characters. I'm voting against encoding specification or > something like that, but I do recommend using the escape method. For > example using "&#e4;" for encoding E4 german ä and < for a "<" > character. This way we can escape all illegal characters. > What do you suggest? > regards, > -Dennis Berger > _______________________________________________ > freebsd-geom@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-geom > To unsubscribe, send any mail to "freebsd-geom-unsubscribe@freebsd.org" > ä of cause!
* Dennis Berger <db@nipsi.de> [2007-03-06 16:53:44 +0100]: > This way we can escape all illegal characters. > What do you suggest? The gstat and other geom applications basically use expat to parse the equivalent of "sysctl -b kern.geom.confxml". This output does not have an encoding specified, so expat accepts only ASCII. As such, bytes greater than 0x7e must be encoded. http://skew.org/xml/tutorial/ http://www.w3.org/TR/1998/REC-xml-19980210 Attached is a patch which attempts to output valid XML for all cases (any value between 0x00 and 0xff). One issue is that many bytes between 0x00 and 0x1f have no valid XML coding, so this patch replaces them with '?' (such things should not appear in geom names, though). Also, it seems that expat is attempting to convert bytes from iso-8859-1 into utf8 characters, so gstat and glabel output may look weird. - doug
Yes I found this version much better than the one before. regards, -Dennis doug steinwand schrieb: > * Dennis Berger <db@nipsi.de> [2007-03-06 16:53:44 +0100]: > >> This way we can escape all illegal characters. >> What do you suggest? >> > > The gstat and other geom applications basically use expat to parse > the equivalent of "sysctl -b kern.geom.confxml". This output does > not have an encoding specified, so expat accepts only ASCII. As > such, bytes greater than 0x7e must be encoded. > > http://skew.org/xml/tutorial/ > http://www.w3.org/TR/1998/REC-xml-19980210 > > Attached is a patch which attempts to output valid XML for all cases > (any value between 0x00 and 0xff). One issue is that many bytes > between 0x00 and 0x1f have no valid XML coding, so this patch > replaces them with '?' (such things should not appear in geom names, > though). > > Also, it seems that expat is attempting to convert bytes from > iso-8859-1 into utf8 characters, so gstat and glabel output may > look weird. > > - doug > > ------------------------------------------------------------------------ > > _______________________________________________ > freebsd-geom@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-geom > To unsubscribe, send any mail to "freebsd-geom-unsubscribe@freebsd.org" -- Dennis Berger BSDSystems Eduardstrasse 43b 20257 Hamburg Phone: +49 (0)40 54 00 18 17 Mobile: +49 (0) 179 123 15 09 E-Mail: db@bsdsystems.de
Any word on this ? some kind of time-out ? -- ------------------------------------------------------------------------ Philip M. Gollucci (pgollucci@p6m7g8.com) 323.219.4708 Consultant / http://p6m7g8.net/Resume Senior Software Engineer - TicketMaster - http://ticketmaster.com 1024D/EC88A0BF 0DE5 C55C 6BF3 B235 2DAB B89E 1324 9B4F EC88 A0BF Work like you don't need the money, love like you'll never get hurt, and dance like nobody's watching.
Is this fixed? Any comment from a committer? regards, Dennis
Hi! This it still the problem for 8.0-RC1 and patch-3 really helps. Please note, that localized versions of Windows assing to newly formatted USB flash drives the label "NEW VOLUME" translated to national language. So, inserting such drive instantly breaks geom utilities until drive removed. patch-3.diff in the PR solves the problem. Eugene Grosbein
Hi, I have updated Doug's patch to use a sbuf instead of allocating the buffer by hand. %%% Index: sys/geom/geom_dump.c =================================================================== --- sys/geom/geom_dump.c (revision 204950) +++ sys/geom/geom_dump.c (working copy) @@ -154,6 +154,29 @@ g_conftxt(void *p, int flag) static void +g_conf_print_encoded(struct sbuf *sb, const char *fmt, const char *str) +{ + struct sbuf *s; + const u_char *c; + + s = sbuf_new_auto(); + + for (c = str; *c != '\0'; c++) { + if (*c == '&' || *c == '<' || *c == '>' || + *c == '\'' || *c == '"' || *c > 0x7e) + sbuf_printf(s, "&#x%X;", *c); + else if (*c == '\t' || *c == '\n' || *c == '\r' || *c > 0x1f) + sbuf_putc(s, *c); + else + sbuf_putc(s, '?'); + } + + sbuf_finish(s); + sbuf_printf(sb, fmt, sbuf_data(s)); + sbuf_delete(s); +} + +static void g_conf_consumer(struct sbuf *sb, struct g_consumer *cp) { @@ -181,7 +204,7 @@ g_conf_provider(struct sbuf *sb, struct sbuf_printf(sb, "\t <geom ref=\"%p\"/>\n", pp->geom); sbuf_printf(sb, "\t <mode>r%dw%de%d</mode>\n", pp->acr, pp->acw, pp->ace); - sbuf_printf(sb, "\t <name>%s</name>\n", pp->name); + g_conf_print_encoded(sb, "\t <name>%s</name>\n", pp->name); sbuf_printf(sb, "\t <mediasize>%jd</mediasize>\n", (intmax_t)pp->mediasize); sbuf_printf(sb, "\t <sectorsize>%u</sectorsize>\n", pp->sectorsize); @@ -208,7 +231,7 @@ g_conf_geom(struct sbuf *sb, struct g_ge sbuf_printf(sb, " <geom id=\"%p\">\n", gp); sbuf_printf(sb, " <class ref=\"%p\"/>\n", gp->class); - sbuf_printf(sb, " <name>%s</name>\n", gp->name); + g_conf_print_encoded(sb, " <name>%s</name>\n", gp->name); sbuf_printf(sb, " <rank>%d</rank>\n", gp->rank); if (gp->flags & G_GEOM_WITHER) sbuf_printf(sb, " <wither/>\n"); @@ -237,7 +260,7 @@ g_conf_class(struct sbuf *sb, struct g_c struct g_geom *gp2; sbuf_printf(sb, " <class id=\"%p\">\n", mp); - sbuf_printf(sb, " <name>%s</name>\n", mp->name); + g_conf_print_encoded(sb, " <name>%s</name>\n", mp->name); LIST_FOREACH(gp2, &mp->geom, geom) { if (gp != NULL && gp != gp2) continue; %%%
Responsible Changed From-To: freebsd-geom->jh Take.
Author: jh Date: Sat Mar 20 16:16:13 2010 New Revision: 205385 URL: http://svn.freebsd.org/changeset/base/205385 Log: Escape characters unsafe for XML output in GEOM class, instance and provider names. - Characters in range 0x01-0x1f except '\t', '\n', and '\r' are replaced with '?'. Those characters are disallowed in XML. - '&', '<', '>', '\'', '"' and characters in range 0x7f-0xff are replaced with XML numeric character reference. If the kern.geom.confxml sysctl provides invalid XML, libgeom geom_xml2tree() fails and utilities using it do not work. Unsafe characters are common in msdosfs and cd9660 labels. PR: kern/104389 Submitted by: Doug Steinwand (original version) Reviewed by: pjd Discussed on: freebsd-geom MFC after: 3 weeks Modified: head/sys/geom/geom_dump.c Modified: head/sys/geom/geom_dump.c ============================================================================== --- head/sys/geom/geom_dump.c Sat Mar 20 15:30:26 2010 (r205384) +++ head/sys/geom/geom_dump.c Sat Mar 20 16:16:13 2010 (r205385) @@ -154,6 +154,28 @@ g_conftxt(void *p, int flag) static void +g_conf_print_escaped(struct sbuf *sb, const char *fmt, const char *str) +{ + struct sbuf *s; + const u_char *c; + + s = sbuf_new_auto(); + + for (c = str; *c != '\0'; c++) { + if (*c == '&' || *c == '<' || *c == '>' || + *c == '\'' || *c == '"' || *c > 0x7e) + sbuf_printf(s, "&#x%X;", *c); + else if (*c == '\t' || *c == '\n' || *c == '\r' || *c > 0x1f) + sbuf_putc(s, *c); + else + sbuf_putc(s, '?'); + } + sbuf_finish(s); + sbuf_printf(sb, fmt, sbuf_data(s)); + sbuf_delete(s); +} + +static void g_conf_consumer(struct sbuf *sb, struct g_consumer *cp) { @@ -181,7 +203,7 @@ g_conf_provider(struct sbuf *sb, struct sbuf_printf(sb, "\t <geom ref=\"%p\"/>\n", pp->geom); sbuf_printf(sb, "\t <mode>r%dw%de%d</mode>\n", pp->acr, pp->acw, pp->ace); - sbuf_printf(sb, "\t <name>%s</name>\n", pp->name); + g_conf_print_escaped(sb, "\t <name>%s</name>\n", pp->name); sbuf_printf(sb, "\t <mediasize>%jd</mediasize>\n", (intmax_t)pp->mediasize); sbuf_printf(sb, "\t <sectorsize>%u</sectorsize>\n", pp->sectorsize); @@ -208,7 +230,7 @@ g_conf_geom(struct sbuf *sb, struct g_ge sbuf_printf(sb, " <geom id=\"%p\">\n", gp); sbuf_printf(sb, " <class ref=\"%p\"/>\n", gp->class); - sbuf_printf(sb, " <name>%s</name>\n", gp->name); + g_conf_print_escaped(sb, " <name>%s</name>\n", gp->name); sbuf_printf(sb, " <rank>%d</rank>\n", gp->rank); if (gp->flags & G_GEOM_WITHER) sbuf_printf(sb, " <wither/>\n"); @@ -237,7 +259,7 @@ g_conf_class(struct sbuf *sb, struct g_c struct g_geom *gp2; sbuf_printf(sb, " <class id=\"%p\">\n", mp); - sbuf_printf(sb, " <name>%s</name>\n", mp->name); + g_conf_print_escaped(sb, " <name>%s</name>\n", mp->name); LIST_FOREACH(gp2, &mp->geom, geom) { if (gp != NULL && gp != gp2) continue; _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
State Changed From-To: open->patched Patched in head (r205385).
Author: jh Date: Sat Apr 10 14:28:58 2010 New Revision: 206458 URL: http://svn.freebsd.org/changeset/base/206458 Log: MFC r205385: Escape characters unsafe for XML output in GEOM class, instance and provider names. - Characters in range 0x01-0x1f except '\t', '\n', and '\r' are replaced with '?'. Those characters are disallowed in XML. - '&', '<', '>', '\'', '"' and characters in range 0x7f-0xff are replaced with XML numeric character reference. If the kern.geom.confxml sysctl provides invalid XML, libgeom geom_xml2tree() fails and utilities using it do not work. Unsafe characters are common in msdosfs and cd9660 labels. PR: kern/104389 Modified: stable/8/sys/geom/geom_dump.c Directory Properties: stable/8/sys/ (props changed) stable/8/sys/amd64/include/xen/ (props changed) stable/8/sys/cddl/contrib/opensolaris/ (props changed) stable/8/sys/contrib/dev/acpica/ (props changed) stable/8/sys/contrib/pf/ (props changed) stable/8/sys/dev/xen/xenpci/ (props changed) Modified: stable/8/sys/geom/geom_dump.c ============================================================================== --- stable/8/sys/geom/geom_dump.c Sat Apr 10 13:54:00 2010 (r206457) +++ stable/8/sys/geom/geom_dump.c Sat Apr 10 14:28:58 2010 (r206458) @@ -154,6 +154,28 @@ g_conftxt(void *p, int flag) static void +g_conf_print_escaped(struct sbuf *sb, const char *fmt, const char *str) +{ + struct sbuf *s; + const u_char *c; + + s = sbuf_new_auto(); + + for (c = str; *c != '\0'; c++) { + if (*c == '&' || *c == '<' || *c == '>' || + *c == '\'' || *c == '"' || *c > 0x7e) + sbuf_printf(s, "&#x%X;", *c); + else if (*c == '\t' || *c == '\n' || *c == '\r' || *c > 0x1f) + sbuf_putc(s, *c); + else + sbuf_putc(s, '?'); + } + sbuf_finish(s); + sbuf_printf(sb, fmt, sbuf_data(s)); + sbuf_delete(s); +} + +static void g_conf_consumer(struct sbuf *sb, struct g_consumer *cp) { @@ -181,7 +203,7 @@ g_conf_provider(struct sbuf *sb, struct sbuf_printf(sb, "\t <geom ref=\"%p\"/>\n", pp->geom); sbuf_printf(sb, "\t <mode>r%dw%de%d</mode>\n", pp->acr, pp->acw, pp->ace); - sbuf_printf(sb, "\t <name>%s</name>\n", pp->name); + g_conf_print_escaped(sb, "\t <name>%s</name>\n", pp->name); sbuf_printf(sb, "\t <mediasize>%jd</mediasize>\n", (intmax_t)pp->mediasize); sbuf_printf(sb, "\t <sectorsize>%u</sectorsize>\n", pp->sectorsize); @@ -204,7 +226,7 @@ g_conf_geom(struct sbuf *sb, struct g_ge sbuf_printf(sb, " <geom id=\"%p\">\n", gp); sbuf_printf(sb, " <class ref=\"%p\"/>\n", gp->class); - sbuf_printf(sb, " <name>%s</name>\n", gp->name); + g_conf_print_escaped(sb, " <name>%s</name>\n", gp->name); sbuf_printf(sb, " <rank>%d</rank>\n", gp->rank); if (gp->flags & G_GEOM_WITHER) sbuf_printf(sb, " <wither/>\n"); @@ -233,7 +255,7 @@ g_conf_class(struct sbuf *sb, struct g_c struct g_geom *gp2; sbuf_printf(sb, " <class id=\"%p\">\n", mp); - sbuf_printf(sb, " <name>%s</name>\n", mp->name); + g_conf_print_escaped(sb, " <name>%s</name>\n", mp->name); LIST_FOREACH(gp2, &mp->geom, geom) { if (gp != NULL && gp != gp2) continue; _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Author: jh Date: Thu Apr 22 14:54:54 2010 New Revision: 207065 URL: http://svn.freebsd.org/changeset/base/207065 Log: MFC r205385: Escape characters unsafe for XML output in GEOM class, instance and provider names. - Characters in range 0x01-0x1f except '\t', '\n', and '\r' are replaced with '?'. Those characters are disallowed in XML. - '&', '<', '>', '\'', '"' and characters in range 0x7f-0xff are replaced with XML numeric character reference. If the kern.geom.confxml sysctl provides invalid XML, libgeom geom_xml2tree() fails and utilities using it do not work. Unsafe characters are common in msdosfs and cd9660 labels. PR: kern/104389 Modified: stable/7/sys/geom/geom_dump.c Directory Properties: stable/7/sys/ (props changed) stable/7/sys/cddl/contrib/opensolaris/ (props changed) stable/7/sys/contrib/dev/acpica/ (props changed) stable/7/sys/contrib/pf/ (props changed) Modified: stable/7/sys/geom/geom_dump.c ============================================================================== --- stable/7/sys/geom/geom_dump.c Thu Apr 22 14:11:59 2010 (r207064) +++ stable/7/sys/geom/geom_dump.c Thu Apr 22 14:54:54 2010 (r207065) @@ -154,6 +154,28 @@ g_conftxt(void *p, int flag) static void +g_conf_print_escaped(struct sbuf *sb, const char *fmt, const char *str) +{ + struct sbuf *s; + const u_char *c; + + s = sbuf_new_auto(); + + for (c = str; *c != '\0'; c++) { + if (*c == '&' || *c == '<' || *c == '>' || + *c == '\'' || *c == '"' || *c > 0x7e) + sbuf_printf(s, "&#x%X;", *c); + else if (*c == '\t' || *c == '\n' || *c == '\r' || *c > 0x1f) + sbuf_putc(s, *c); + else + sbuf_putc(s, '?'); + } + sbuf_finish(s); + sbuf_printf(sb, fmt, sbuf_data(s)); + sbuf_delete(s); +} + +static void g_conf_consumer(struct sbuf *sb, struct g_consumer *cp) { @@ -181,7 +203,7 @@ g_conf_provider(struct sbuf *sb, struct sbuf_printf(sb, "\t <geom ref=\"%p\"/>\n", pp->geom); sbuf_printf(sb, "\t <mode>r%dw%de%d</mode>\n", pp->acr, pp->acw, pp->ace); - sbuf_printf(sb, "\t <name>%s</name>\n", pp->name); + g_conf_print_escaped(sb, "\t <name>%s</name>\n", pp->name); sbuf_printf(sb, "\t <mediasize>%jd</mediasize>\n", (intmax_t)pp->mediasize); sbuf_printf(sb, "\t <sectorsize>%u</sectorsize>\n", pp->sectorsize); @@ -204,7 +226,7 @@ g_conf_geom(struct sbuf *sb, struct g_ge sbuf_printf(sb, " <geom id=\"%p\">\n", gp); sbuf_printf(sb, " <class ref=\"%p\"/>\n", gp->class); - sbuf_printf(sb, " <name>%s</name>\n", gp->name); + g_conf_print_escaped(sb, " <name>%s</name>\n", gp->name); sbuf_printf(sb, " <rank>%d</rank>\n", gp->rank); if (gp->flags & G_GEOM_WITHER) sbuf_printf(sb, " <wither/>\n"); @@ -233,7 +255,7 @@ g_conf_class(struct sbuf *sb, struct g_c struct g_geom *gp2; sbuf_printf(sb, " <class id=\"%p\">\n", mp); - sbuf_printf(sb, " <name>%s</name>\n", mp->name); + g_conf_print_escaped(sb, " <name>%s</name>\n", mp->name); LIST_FOREACH(gp2, &mp->geom, geom) { if (gp != NULL && gp != gp2) continue; _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
State Changed From-To: patched->closed Fixed in head, stable/8 and stable/7.