Labels from external volumes may contain characters that make the label difficult to use for practical purposes. An example is a MSDOS filesystem with whitespace in its label; an fstab entry can't be written for this volume as /etc/fstab has a strict and simple syntax that allows for space only between fields. Fix: Although there have been already several proposals, in the past decade, to enhance fstab's syntax (see conf/37569, bin/55539 and bin/117687, conf/149424), this one takes a novel approach: labels are sanitised before creating the entry in /dev/, thus removing the requirement for a new syntax in /etc/fstab In the following patch, the function sanitise_name (controlled by kern.geom.label.sanitation sysctl) replaces characters deemed invalid according to three levels: 0 - replace only '/'s with '#'s 1 - like 0, but also replace whitespace and all the ASCII control characters (anything below 0x20) with '_' 2 - like 1, but also replace anything above 0x7E with '#' Note that the replacement of '/'s is done in any case. I don't know whether that might be redundant or even bad in the context of the other geom_label stuff. It just made sense, but I don't have enough familiarity with that code. =================================================================== RCS file: /repos/src/sys/geom/label/g_label.c,v retrieving revision 1.24.2.4 How-To-Repeat: Plug a USB dongle with a volume that contains whitespace in the name, such as "foo bar". Now try to write an entry in /etc/fstab for it.
The patch submitted just before contained a bug. Please disregard and consider the following instead: *** g_label.c 22 Jun 2010 08:17:20 -0000 1.24.2.4 --- g_label.c 18 Aug 2010 10:56:09 -0000 *************** *** 136,141 **** --- 136,161 ---- return (1); } + static int sanitation_level = 1; + SYSCTL_INT(_kern_geom_label, OID_AUTO, sanitation, CTLFLAG_RW, + &sanitation_level, 0, + "Correction applied to labels: 0 = replace '/'s only, 1 = '/' + whitespace and ctrls, 2 = '/' + anything but ASCII printables"); + + static void + sanitise_name (char *name) + { + char *p; + + for (p = name; *p; ++p) { + if (*p == '/') + *p = '#'; + else if (sanitation_level > 0 && *p <= ' ') + *p = '_'; + else if (sanitation_level > 1 && *p > '~') + *p = '#'; + } + } + static struct g_geom * g_label_create(struct gctl_req *req, struct g_class *mp, struct g_provider *pp, const char *label, const char *dir, off_t mediasize) *************** *** 156,161 **** --- 176,182 ---- gp = NULL; cp = NULL; snprintf(name, sizeof(name), "%s/%s", dir, label); + sanitise_name(name + strlen(dir) + 1); LIST_FOREACH(gp, &mp->geom, geom) { pp2 = LIST_FIRST(&gp->provider); if (pp2 == NULL) -- walter pelissero http://www.pelissero.de
State Changed From-To: open->closed Unless you properly fit yourself in the community I am not even willing to look into your issue. Goodbye.
Walter C. Pelissero wrote: > In the following patch, the function sanitise_name (controlled > by kern.geom.label.sanitation sysctl) replaces characters > deemed invalid according to three levels: > > 0 - replace only '/'s with '#'s > 1 - like 0, but also replace whitespace and all the ASCII control > characters (anything below 0x20) with '_' > 2 - like 1, but also replace anything above 0x7E with '#' > > Note that the replacement of '/'s is done in any case. I > don't know whether that might be redundant or even bad in the > context of the other geom_label stuff. It just made sense, > but I don't have enough familiarity with that code. FWIW, I like this approach much better than the one from bin/149424. (Trying to put personal attitudes aside.) I have reviewed the patch (the second one from the followup), and it looks good to me, except for a few minor style(9) issues, and one type problem: By default, char is unsigned, so the comparisons p <= ' ' and p > '~' probably don't do what you expect. Other than that, the patch looks fine to me. But I'm not authoritative regarding the geom code in the kernel, so I cannot pick this up myself. I'll re-assign the PR to -geom, maybe someone there can help. Best regards Oliver -- Oliver Fromme, secnetix GmbH & Co. KG, Marktplatz 29, 85567 Grafing b. M. Handelsregister: Registergericht Muenchen, HRA 74606, Geschäftsfuehrung: secnetix Verwaltungsgesellsch. mbH, Handelsregister: Registergericht Mün- chen, HRB 125758, Geschäftsführer: Maik Bachmann, Olaf Erb, Ralf Gebhart FreeBSD-Dienstleistungen, -Produkte und mehr: http://www.secnetix.de/bsd "... there are two ways of constructing a software design: One way is to make it so simple that there are _obviously_ no deficiencies and the other way is to make it so complicated that there are no _obvious_ deficiencies." -- C.A.R. Hoare, ACM Turing Award Lecture, 1980
State Changed From-To: closed->open Re-open and assign to -geom. Maybe some knowledgeable person here can review and handle this.
Responsible Changed From-To: freebsd-bugs->freebsd-geom Re-open and assign to -geom. Maybe some knowledgeable person here can review and handle this.
Oliver Fromme wrote: > I have reviewed the patch (the second one from the followup), > and it looks good to me, except for a few minor style(9) > issues, and one type problem: By default, char is unsigned, ^^^^^^^^^^^^^^^^^^ > so the comparisons p <= ' ' and p > '~' probably don't do > what you expect. I'm sorry, I meant signed, of course. You need to specify "unsigned char" if you want it to be unsigned. Best regards Oliver -- Oliver Fromme, secnetix GmbH & Co. KG, Marktplatz 29, 85567 Grafing b. M. Handelsregister: Registergericht Muenchen, HRA 74606, Geschäftsfuehrung: secnetix Verwaltungsgesellsch. mbH, Handelsregister: Registergericht Mün- chen, HRB 125758, Geschäftsführer: Maik Bachmann, Olaf Erb, Ralf Gebhart FreeBSD-Dienstleistungen, -Produkte und mehr: http://www.secnetix.de/bsd "C is quirky, flawed, and an enormous success." -- Dennis M. Ritchie.
On Wed, Aug 18, 2010 at 07:06:26PM +0200, Oliver Fromme wrote: > Oliver Fromme wrote: > > I have reviewed the patch (the second one from the followup), > > and it looks good to me, except for a few minor style(9) > > issues, and one type problem: By default, char is unsigned, > ^^^^^^^^^^^^^^^^^^ > > so the comparisons p <= ' ' and p > '~' probably don't do > > what you expect. > > I'm sorry, I meant signed, of course. > You need to specify "unsigned char" if you want it to be unsigned. Signedness of char is implementation-depended. It is signed on x86-oids, and unsigned on powerpc, AFAIR.
This is a hack, something that you would commonly find in Linux code and is neither a proper or viable workaround for the naming of labels. Instead, using glabel(8) the admin/user can create a local label to FreeBSD that does not change the original nor does it carry over to any other OS that does not understand geom_label's. From the manual page: label Set up a label name for the given provider. This is the ``automatic'' method, where metadata is stored in a provider's last sector. The kernel module geom_label.ko will be loaded if it is not loaded already. Stripping characters no matter what they are with a sysctl is overkill and does not scale well, all the while - presenting false information to the user. I would highly advise against this. If the user does not like the label that appears in msdosfs/{LABEL} then they are free to change that at their own will. I see presenting the label as it is to the user ``important''. Lets not try to prevent a foot shooting but instead document the case in the manual. Also if they are using FreeBSD and they are also adding a mount-point to fstab(5) for a geom_label then intelligence lurks within. Regards, -- jhell,v
jhell <jhell@dataix.net> wrote: > This is a hack, something that you would commonly find in Linux code and > is neither a proper or viable workaround for the naming of labels. > > Instead, using glabel(8) the admin/user can create a local label to > FreeBSD that does not change the original nor does it carry over to any > other OS that does not understand geom_label's. There are cases were you cannot create a permanent label with glabel. For example, there are quite a lot of USB devices that insist on having their memory formatted with their own firmware only, destroying any other labels. I own an mp3 player (Medion) that behaves like this. The submitter mentioned a digital camera (Nikon, I think) in a previous thread. How do you suggest to solve the problem for those cases? > Stripping characters no matter what they are with a sysctl is overkill > and does not scale well, all the while - presenting false information to > the user. I don't think there is a scalability issue, because it is only done once when a device is attached. On the other hand, maybe the patch should be changed so it doesn't touch the name at all when the sysctl is 0, so the default behaviour would be no different from what we have today. > I would highly advise against this. If the user does not like > the label that appears in msdosfs/{LABEL} then they are free to change > that at their own will. Unfortunately they are subject to the will of their devices, see above. And if the device's own label contains a space (the Nikon camera mentioned above does), it cannot be used as-is in /etc/fstab. I think a similar problem occurs with the volume names of CD-ROMs and DVD-ROMs, which contain spaces quite often, and you cannot glabel them. But I haven't looked deeply into this, maybe it's not an issue. (Does FreeBSD even support mounting CDs by their volume name, like Solaris does? I've never tried, but it would come in handy, because I have a clip art CD that I use every now and then, and I never remember whether it's in drive cd0 or cd1 currently ...) > I see presenting the label as it is to the user > ``important''. Uhm ... it is not my impression that the names of nodes in /dev are intended to present accurate "external" information to the user. I always thought that the purpose of /dev is an interface between userland software and drivers, and that users shouldn't have to deal with /dev at all during "normal operations". Am I wrong? I mean, there are commands to display the atual labels if you need to see them (and I don't mean "ls /dev/..."). Best regards Oliver -- Oliver Fromme, secnetix GmbH & Co. KG, Marktplatz 29, 85567 Grafing b. M. Handelsregister: Registergericht Muenchen, HRA 74606, Geschäftsfuehrung: secnetix Verwaltungsgesellsch. mbH, Handelsregister: Registergericht Mün- chen, HRB 125758, Geschäftsführer: Maik Bachmann, Olaf Erb, Ralf Gebhart FreeBSD-Dienstleistungen, -Produkte und mehr: http://www.secnetix.de/bsd "When your hammer is C++, everything begins to look like a thumb." -- Steve Haflich, in comp.lang.c++
jhell writes: > This is a hack, something that you would commonly find in Linux > code and is neither a proper or viable workaround for the naming of > labels. That's exactly what I keep repeating my friends who run Linux and brag about features they had first or better implemented. Just a bunch of dirty hacks any kid would have whipped up in an afternoon if he just had the fancy. Just hacks. Unfortunately they answer using terms like: self-indulgent inertia, inconclusive arrogance, bureaucratic attitude, self-important sloth, pompous underachiever. Bloody Linux kids, aren't they? This might be an hack, but in 8 years since this issue has been first raised nobody has came up with it, so I thought I might as well do it. > Instead, using glabel(8) the admin/user can create a local label to > FreeBSD that does not change the original nor does it carry over to any > other OS that does not understand geom_label's. > > >From the manual page: > > label Set up a label name for the given provider. This is the > ``automatic'' method, where metadata is stored in a provider's > last sector. The kernel module geom_label.ko will be loaded if > it is not loaded already. Can anyone explain me how metadata stored on media cannot interfere with other OSs? Below I include a revised patch that turns kern.geom.label.sanitation into a bitmask, thus allowing to enable selectively the replacements, as I have been struck by the fact that slashes in labels might be desirable, as they create a path (although the parent directory is left behind after the device disappearance). Now kern.geom.label.sanitation can be set to zero (default) obtaining the usual behaviour without "hack". So that when I meet up with some Vogon friends, who coincidentally all run exclusively FreeBSD or VMS, I can revert the sysctl and show them their volume labels coded in UTF-16. Well, yes, "show". Typing those labels at the shell prompt is another story altogether, but at least they won't scoff at me as if I was a Linux hacker! Index: g_label.c =================================================================== RCS file: /repos/src/sys/geom/label/g_label.c,v retrieving revision 1.24.2.4 diff -c -r1.24.2.4 g_label.c *** g_label.c 22 Jun 2010 08:17:20 -0000 1.24.2.4 --- g_label.c 19 Aug 2010 11:27:13 -0000 *************** *** 136,141 **** --- 136,161 ---- return (1); } + static int sanitation_mask = 0; + SYSCTL_INT(_kern_geom_label, OID_AUTO, sanitation, CTLFLAG_RW, + &sanitation_mask, 0, + "Correction applied to labels. A bitmask of: 0 = none, 1 = replace '/'s, 2 = replace whitespace and ctrls, 4 = replace anything beyond '~'"); + + static void + sanitise_name (char *name) + { + unsigned char *p; + + for (p = name; *p; ++p) { + if ((sanitation_mask & 1) && *p == '/') + *p = '#'; + else if ((sanitation_mask & 2) && *p <= ' ') + *p = '_'; + else if ((sanitation_mask & 4) && *p > '~') + *p = '?'; + } + } + static struct g_geom * g_label_create(struct gctl_req *req, struct g_class *mp, struct g_provider *pp, const char *label, const char *dir, off_t mediasize) *************** *** 156,161 **** --- 176,182 ---- gp = NULL; cp = NULL; snprintf(name, sizeof(name), "%s/%s", dir, label); + sanitise_name(name + strlen(dir) + 1); LIST_FOREACH(gp, &mp->geom, geom) { pp2 = LIST_FIRST(&gp->provider); if (pp2 == NULL) -- walter pelissero http://www.pelissero.de
On 08/19/2010 07:29, Oliver Fromme wrote: > jhell <jhell@dataix.net> wrote: > > This is a hack, something that you would commonly find in Linux code and > > is neither a proper or viable workaround for the naming of labels. > > > > Instead, using glabel(8) the admin/user can create a local label to > > FreeBSD that does not change the original nor does it carry over to any > > other OS that does not understand geom_label's. > > There are cases were you cannot create a permanent label > with glabel. For example, there are quite a lot of USB > devices that insist on having their memory formatted with > their own firmware only, destroying any other labels. > I own an mp3 player (Medion) that behaves like this. > The submitter mentioned a digital camera (Nikon, I think) > in a previous thread. > > How do you suggest to solve the problem for those cases? > > > Stripping characters no matter what they are with a sysctl is overkill > > and does not scale well, all the while - presenting false information to > > the user. > > I don't think there is a scalability issue, because it is > only done once when a device is attached. On the other > hand, maybe the patch should be changed so it doesn't > touch the name at all when the sysctl is 0, so the default > behaviour would be no different from what we have today. If this is the case then why adapt geom_label at all ? why not instead go straight for the culprit "devfs/devd" and add a sysctl for stripping what needs to be stripped ?. This way it is distinct to /dev and can be done on the system-wide basis and/or focused on particular pieces of the system like geom_label. This is more of what I meant by scalable. > > > I would highly advise against this. If the user does not like > > the label that appears in msdosfs/{LABEL} then they are free to change > > that at their own will. > > Unfortunately they are subject to the will of their devices, > see above. And if the device's own label contains a space > (the Nikon camera mentioned above does), it cannot be used > as-is in /etc/fstab. That is understandable but to adjust the code to fit a few edge cases like this IMO just feels wrong, remember this is only my opinion in this case. > > I think a similar problem occurs with the volume names of > CD-ROMs and DVD-ROMs, which contain spaces quite often, > and you cannot glabel them. But I haven't looked deeply > into this, maybe it's not an issue. > > (Does FreeBSD even support mounting CDs by their volume > name, like Solaris does? I've never tried, but it would > come in handy, because I have a clip art CD that I use > every now and then, and I never remember whether it's in > drive cd0 or cd1 currently ...) Yes it does. I usually turn these off for client systems as I find them very unneeded and duplicate cases that can cause hell on hald(8). Design flaw of hald(8)? no but we introduced it. kern.geom.label.ext2fs.enable=0 kern.geom.label.iso9660.enable=0 kern.geom.label.msdosfs.enable=0 kern.geom.label.ntfs.enable=0 kern.geom.label.reiserfs.enable=0 kern.geom.label.ufs.enable=0 kern.geom.label.ufsid.enable=0 kern.geom.label.gptid.enable=0 kern.geom.label.gpt.enable=0 > > > I see presenting the label as it is to the user > > ``important''. > > Uhm ... it is not my impression that the names of nodes > in /dev are intended to present accurate "external" > information to the user. I always thought that the > purpose of /dev is an interface between userland software > and drivers, and that users shouldn't have to deal with > /dev at all during "normal operations". Am I wrong? No you are not wrong. But in the case of first introduction of the device and the use of fstab(5) being mentioned already, the user has to interact with /dev right ? > > I mean, there are commands to display the atual labels if > you need to see them (and I don't mean "ls /dev/..."). > Right I agree with this to an extent that the hypothetical user should not have to drop back to a manual page to find out how to get the original information of the device. Regards, -- jhell,v
On 08/19/2010 08:26, Walter C. Pelissero wrote: > jhell writes: > > This is a hack, something that you would commonly find in Linux > > code and is neither a proper or viable workaround for the naming of > > labels. > > That's exactly what I keep repeating my friends who run Linux and brag > about features they had first or better implemented. Just a bunch of > dirty hacks any kid would have whipped up in an afternoon if he just > had the fancy. Just hacks. > Putting aside whats written below, please don't feel that this is an attack on your work to make this happen. I feel your work is very viable for what it is supposed to do but to the same effect does not address the issue that this might happen elsewhere to anything new that may be introduced to the system at a later point in time which is why I call it a hack. In a previous post I mentioned a proper full term solution based on adjusting devfs/devd instead that would fit future situations and not call for individual adjustments for pieces of geom or anything related. > Unfortunately they answer using terms like: self-indulgent inertia, > inconclusive arrogance, bureaucratic attitude, self-important sloth, > pompous underachiever. Bloody Linux kids, aren't they? > > This might be an hack, but in 8 years since this issue has been first > raised nobody has came up with it, so I thought I might as well do > it. > It has be ~6 years that GEOM_LABEL was introduced to the system. r131476 | pjd | 2004-07-02 15:40:36 -0400 (Fri, 02 Jul 2004) | 22 lines Regards, -- jhell,v
jhell writes: > Putting aside whats written below, please don't feel that this is an > attack on your work to make this happen. Attack on my work? Amusing. > I feel your work is very viable for what it is supposed to do but > to the same effect does not address the issue that this might > happen elsewhere to anything new that may be introduced to the > system at a later point in time What don't you understand in: Now kern.geom.label.sanitation can be set to zero (default) obtaining the usual behaviour without "hack". > In a previous post I mentioned a proper full term solution based on > adjusting devfs/devd instead that would fit future situations and > not call for individual adjustments for pieces of geom or anything > related. I failed to see the devfs/devd solution you mention. > It has be ~6 years that GEOM_LABEL was introduced to the system. > r131476 | pjd | 2004-07-02 15:40:36 -0400 (Fri, 02 Jul 2004) | 22 lines conf/37569 Mon Apr 29 08:40:01 PDT 2002 The common issue of all the already mentioned PRs is: mounting volumes with names that are not compatible with fstab (and a lot of other things). Which this modification of geom_label could mitigate. There is a related PR of mine, which tackles the problem from the fstab stand point, that does more or less what the other PR has been already suggesting since 2002 and Linux has been doing for more than a decade. It was closed by a sulky baby boy on the same grounds as this. -- walter pelissero http://www.pelissero.de
On 08/20/2010 05:58, Walter C. Pelissero wrote: > jhell writes: > > Putting aside whats written below, please don't feel that this is an > > attack on your work to make this happen. > > Attack on my work? > Amusing. > > > > I feel your work is very viable for what it is supposed to do but > > to the same effect does not address the issue that this might > > happen elsewhere to anything new that may be introduced to the > > system at a later point in time > > What don't you understand in: > Now kern.geom.label.sanitation can be set to zero (default) > obtaining the usual behaviour without "hack". I understand that completely still it is not a proper fix for the situation. > > > > In a previous post I mentioned a proper full term solution based on > > adjusting devfs/devd instead that would fit future situations and > > not call for individual adjustments for pieces of geom or anything > > related. > > I failed to see the devfs/devd solution you mention. I believe I meant patch devfs/devd here rather than adjust. Since were dealing with what shows up in the device file-system why not patch that instead of horrifying pieces of the system, one at a time. vfs.devfs.``normalization'' - Would probably suit this. I don't expect you to understand this... ;) > > > > It has be ~6 years that GEOM_LABEL was introduced to the system. > > r131476 | pjd | 2004-07-02 15:40:36 -0400 (Fri, 02 Jul 2004) | 22 lines > > conf/37569 > Mon Apr 29 08:40:01 PDT 2002 > > The common issue of all the already mentioned PRs is: mounting volumes > with names that are not compatible with fstab (and a lot of other > things). Which this modification of geom_label could mitigate. > > There is a related PR of mine, which tackles the problem from the > fstab stand point, that does more or less what the other PR has been > already suggesting since 2002 and Linux has been doing for more than a > decade. > > It was closed by a sulky baby boy on the same grounds as this. > Personally I have seen enough of the hash and bash coming from anyone here. Ive tried to make sure that what I have wrote back as a reply to your work was neither demeaning or intrusive but yet you continue to take the same negative standpoint with objection toward further ideas all the while holding an immature context with a submitter. Besides this! It is not really needed to adjust your personal system for the likes of some labels that may contain weird characters that conflict with how the system parses fstab(5). Simply by using the 'link' action in devfs.conf(5) you can create a link to a device auto-magically when the device is plugged in that would be available for use in fstab(5). link label/my?edgecase myedgecase2 link msdosfs/your?edgecase myedgecase3 This is only performed if the device appears. In fstab(5) /dev/myedgecase2 /headcase2 msdosfs rw,noauto 0 0 /dev/myedgecase3 /headcase3 msdosfs rw,noauto 0 0 Of course you can also adjust devd.conf(5) for example to perform specific actions upon device entry but Ill let you figure that out. Regards, -- jhell,v
How about using "percent encoding": http://en.wikipedia.org/wiki/Percent-encoding It is well known and solves all issues.
Marcin Wi=B6nicki writes: > How about using "percent encoding": > http://en.wikipedia.org/wiki/Percent-encoding >=20 > It is well known and solves all issues. Which would have the advantage of being one byte more compact than the backslash-octal encoding. sanitise=5Fname performs a simple masking because the pathname built in= g=5Flabel=5Fcreate is limited to 64 chars, which I didn't know whether = it had some special meaning (on a second look, it seems as it didn't), and it looked rather tight for anything more elaborate. --=20 walter pelissero http://www.pelissero.de
For bugs matching the following criteria: Status: In Progress Changed: (is less than) 2014-06-01 Reset to default assignee and clear in-progress tags. Mail being skipped