kern/149762: volume labels with rogue characters

[Walter C. Pelissero]

>Number:         149762
>Category:       kern
>Synopsis:       volume labels with rogue characters
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          change-request
>Submitter-Id:   current-users
>Arrival-Date:   Wed Aug 18 10:20:08 UTC 2010
>Closed-Date:
>Last-Modified:
>Originator:     Walter C. Pelissero
>Release:        FreeBSD 8.1-STABLE i386
>Organization:
>Environment:
System: FreeBSD zaphod.home.lan 8.1-STABLE FreeBSD 8.1-STABLE #1: Wed Aug 18 10:09:27 CEST 2010 root at zaphod.home.lan:/usr/src/sys/i386/compile/TIGER-MP i386


	
>Description:
	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.
>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.
>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
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	18 Aug 2010 09:48:41 -0000
***************
*** 136,141 ****
--- 136,162 ----
  	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 *dst, const char *src)
+ {
+ 	for (; *src; ++src, ++dst) {
+ 		if (*src == '/')
+ 			*dst = '#';
+ 		else if (sanitation_level > 0 && *src <= ' ')
+ 			*dst = '_';
+ 		else if (sanitation_level > 1 && *src > '~')
+ 			*dst = '#';
+ 		else
+ 			*dst = *src;
+ 	}
+ 	*dst = '\0';
+ }
+ 
  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)
***************
*** 155,161 ****
  	}
  	gp = NULL;
  	cp = NULL;
! 	snprintf(name, sizeof(name), "%s/%s", dir, label);
  	LIST_FOREACH(gp, &mp->geom, geom) {
  		pp2 = LIST_FIRST(&gp->provider);
  		if (pp2 == NULL)
--- 176,185 ----
  	}
  	gp = NULL;
  	cp = NULL;
! 	{
! 		int n = snprintf(name, sizeof(name), "%s/", dir);
! 		sanitise_name(name + n, label);
! 	}
  	LIST_FOREACH(gp, &mp->geom, geom) {
  		pp2 = LIST_FIRST(&gp->provider);
  		if (pp2 == NULL)
>Release-Note:
>Audit-Trail:
>Unformatted: