| Message ID | 20130422150612.GA22006@upset.ux.pdb.fsc.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Andreas, On Mon, 22 Apr 2013, Andreas Friedrich wrote: > Hallo, > > I think I have found a bug in the ceph-0.56.4 start script concerning > the following code section in src/init-ceph.in: > > 212 if [ "$host" = "$hostname" ]; then > 213 cur_conf=$conf > 214 else > 215 unique=`dd if=/dev/urandom bs=16 count=1 2>/dev/null | md5sum | awk '{print $1}'` > 216 if echo $pushed_to | grep -v -q " $host "; then > 217 scp -q $conf $host:/tmp/ceph.conf.$unique > 218 trap "ssh $host rm /tmp/ceph.conf.$unique" EXIT > 219 pushed_to="$pushed_to $host " > 220 fi > 221 cur_conf="/tmp/ceph.conf.$unique" > 222 fi > 223 cmd="$cmd -c $cur_conf" > > Explanation: > If there are more than one OSD instances configured for remote host > "abc", the ceph configuration with a random file name will be copied > to host "abc" and the pushed_to variable will be set. > > Next time, if there is another OSD instance for remote host "abc", > the pushed_to variable is already set and therefore _no_ new ceph > configuration file will be copied to host "abc". That's OK but > nevertheless a new random file name will be created in line 215/221 > and will be used in line 223. This leads to an error because the > new configuration file name is unknown on host "abc". Thanks for the report! I think this doesn't quite solve the problem, though. If all the items for a given target are processed in order, then $unique will be correct, but if you get hosta, hostb, hosta then it will be wrong. The simplist fix is probably to use a single $unique for the lifetime of the script, but this makes the filename predictable to a certain class of resourceful malicious users. Alternatively, we chould only put a single remote target in $pushed_to and re-push the conf to a new temp file if we get the same host again out of order. sage -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mo, Apr 22, 2013 at 08:39:03 -0700, Sage Weil wrote: Hi Sage, ... > Thanks for the report! I think this doesn't quite solve the problem, > though. If all the items for a given target are processed in order, then > $unique will be correct, but if you get hosta, hostb, hosta then it will > be wrong. ... sorry, but I don't recognize why it would be wrong: Applying my patch the code would do the following: 1. hosta: Create ceph.conf.uniq1, copy it to hosta and use ceph.conf.uniq1. 2. hostb: Create ceph.conf.uniq2, copy it to hostb and use ceph.conf.uniq2. 3. hosta: Create ceph.conf.uniq3, copy it to hosta and use ceph.conf.uniq3. So in this example two config files with different names but the same content would be copied to hosta. This is not very pretty but it should work without error. Andreas -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 23 Apr 2013, Andreas Friedrich wrote: > On Mo, Apr 22, 2013 at 08:39:03 -0700, Sage Weil wrote: > > Hi Sage, > > ... > > Thanks for the report! I think this doesn't quite solve the problem, > > though. If all the items for a given target are processed in order, then > > $unique will be correct, but if you get hosta, hostb, hosta then it will > > be wrong. > ... > > sorry, but I don't recognize why it would be wrong: > > Applying my patch the code would do the following: > > 1. hosta: > Create ceph.conf.uniq1, copy it to hosta and use > ceph.conf.uniq1. > > 2. hostb: > Create ceph.conf.uniq2, copy it to hostb and use > ceph.conf.uniq2. > > 3. hosta: > Create ceph.conf.uniq3, copy it to hosta and use > ceph.conf.uniq3. > > So in this example two config files with different names but the same > content would be copied to hosta. This is not very pretty but it > should work without error. It's because of the $pushed_to list.. for #3 it wouldn't re-push ceph.conf. I pushed a patch that just skips that optimization entirely: https://github.com/ceph/ceph/commit/ccbc4dbc6edf09626459ca52a53a72682f541e86 Look okay to you? Thanks! sage -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Di, Apr 23, 2013 at 10:01:53 -0700, Sage Weil wrote: ... > > So in this example two config files with different names but the same > > content would be copied to hosta. This is not very pretty but it > > should work without error. > > It's because of the $pushed_to list.. for #3 it wouldn't re-push > ceph.conf. I pushed a patch that just skips that optimization entirely: > > https://github.com/ceph/ceph/commit/ccbc4dbc6edf09626459ca52a53a72682f541e86 > > Look okay to you? Yes, but I'm not happy with the trap instruction: trap "ssh $host rm /tmp/ceph.conf.$unique" EXIT If the start script exits with or without error, the remote config file will be removed from <remote-host>:/tmp - but only for the last host (the former trap calls will be overwritten by the last trap call)! This makes no sense to me. Best regards Andreas -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff -urN a/src/init-ceph.in b/src/init-ceph.in --- a/src/init-ceph.in 2013-03-25 18:59:06.000000000 +0100 +++ b/src/init-ceph.in 2013-04-22 16:34:02.000000000 +0200 @@ -212,8 +212,8 @@ if [ "$host" = "$hostname" ]; then cur_conf=$conf else - unique=`dd if=/dev/urandom bs=16 count=1 2>/dev/null | md5sum | awk '{print $1}'` if echo $pushed_to | grep -v -q " $host "; then + unique=`dd if=/dev/urandom bs=16 count=1 2>/dev/null | md5sum | awk '{print $1}'` scp -q $conf $host:/tmp/ceph.conf.$unique trap "ssh $host rm /tmp/ceph.conf.$unique" EXIT pushed_to="$pushed_to $host "
Hallo, I think I have found a bug in the ceph-0.56.4 start script concerning the following code section in src/init-ceph.in: 212 if [ "$host" = "$hostname" ]; then 213 cur_conf=$conf 214 else 215 unique=`dd if=/dev/urandom bs=16 count=1 2>/dev/null | md5sum | awk '{print $1}'` 216 if echo $pushed_to | grep -v -q " $host "; then 217 scp -q $conf $host:/tmp/ceph.conf.$unique 218 trap "ssh $host rm /tmp/ceph.conf.$unique" EXIT 219 pushed_to="$pushed_to $host " 220 fi 221 cur_conf="/tmp/ceph.conf.$unique" 222 fi 223 cmd="$cmd -c $cur_conf" Explanation: If there are more than one OSD instances configured for remote host "abc", the ceph configuration with a random file name will be copied to host "abc" and the pushed_to variable will be set. Next time, if there is another OSD instance for remote host "abc", the pushed_to variable is already set and therefore _no_ new ceph configuration file will be copied to host "abc". That's OK but nevertheless a new random file name will be created in line 215/221 and will be used in line 223. This leads to an error because the new configuration file name is unknown on host "abc". The appended patch fixes this misbehavior. With best regards, Andreas Friedrich ---------------------------------------------------------------------- FUJITSU Fujitsu Technology Solutions GmbH Heinz-Nixdorf-Ring 1, 33106 Paderborn, Germany Tel: +49 (5251) 525-1512 Fax: +49 (5251) 525-321512 Email: andreas.friedrich@ts.fujitsu.com Web: ts.fujitsu.com Company details: de.ts.fujitsu.com/imprint ----------------------------------------------------------------------