Date: Mon, 12 Jan 2009 10:44:25 +0100 From: Christoph Mallon <christoph.mallon@gmx.de> To: obrien@freebsd.org Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r186504 - head/sbin/mount Message-ID: <496B10F9.20201@gmx.de> In-Reply-To: <20090112082510.GA69194@dragon.NUXI.org> References: <200812262254.mBQMsrbR052676@svn.freebsd.org> <4960FA9A.1090509@gmx.de> <20090111041543.GB17602@dragon.NUXI.org> <4969A626.6070908@gmx.de> <20090112082510.GA69194@dragon.NUXI.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format. --------------050908040806000001080607 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit David O'Brien schrieb: > On Sun, Jan 11, 2009 at 08:56:22AM +0100, Christoph Mallon wrote: >> David O'Brien schrieb: >>> On Sun, Jan 04, 2009 at 07:06:18PM +0100, Christoph Mallon wrote: >>>> I'm pretty sure $SUPERNATURAL_BEING_OF_YOUR_CHOICE killed a kitten for >>>> the ugly hack you added to mount. The moment you overflow a buffer, you >>>> are in no man's land and there's no escape. I appended a patch, which >>>> solves this issue once and for all: The argv array gets dynamically >>>> expanded, when its limit is reached. >>>> Please - for all kittens out there - commit this patch. >>> Hi Christoph, >>> Unfortunately your patch doesn't work. >>> For a 'ufs' file system listed in /etc/fstab >>> $ umount /foo >>> $ mount /foo >>> Does not work. >> Why haven't you told me earlier? > > Wow, wish I did have the gift to know something before I know something. > Especially tomorrow's lotto #'s. ;-) I sent you my patch almost a week before you commited your changes. I think, there was enough time to tell me, that my patch had a flaw. Alternatively I would have gladly reviewed your solution. >> It was a trivial glitch - just a missing >> "--mnt_argc;". I would've corrected it right away. I tested with a CD and >> this takes a different code path, which does not trigger the problem. >> Attached is the corrected patch. > > Actually this version doesn't work either. > Trying to mount root from ufs:/dev/ad4s1a > <fsck of all UFS's in /etc/fstab> > .. > usage: mount [-t fstype] [-o options] target_fs mount_point > Mounting /etc/fstab filesystems failed, startup aborted > ERROR: ABORTING BOOT (sending SIGTERM to parent)! > > Your over use of global variables made my adult cat cry in pain. > Too bad you didn't at least follow the lead of what I committed. > > I've committed something in the spirit of your patches. I hope > that suits everyone. Actually this version doesn't work either. tron# ./mount -a usage: mount [-t fstype] [-o options] target_fs mount_point tron# ./mount -d -a mount -t ufs -o rw -o update /dev/ad0s1a / mount -t ufs ��`X (濿��`X /dev/ad0s1f /data (Sorry for the garbage, it actually printed that. You can turn it into a "clean" segfault by memset()ing mnt_argv just after its declaration) Your incorrect use of local variables and the resulting undefined behaviour made the cat, who visits me on a roof behind the house sometimes, almost fall from said roof, when he saw your commit: You expect the local variable "struct cpa mnt_argv" still to have the same values after mountfs() was left and reentered. Too bad you didn't at least follow the lead of what I proposed. I've attached a corrected version of my patch, which has a mnt_argc = 0; added in order to reset the argument vector on reentry of mountfs() (instead of appending to the arguments of the last round). --------------050908040806000001080607 Content-Type: text/plain; name="mount.diff" Content-Transfer-Encoding: base64 Content-Disposition: inline; filename="mount.diff" SW5kZXg6IG1vdW50LmMKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gbW91bnQuYwkoUmV2aXNpb24gMTg3 MDkzKQorKysgbW91bnQuYwkoQXJiZWl0c2tvcGllKQpAQCAtNjcsMTggKzY3LDE2IEBACiAj ZGVmaW5lIE1PVU5UX01FVEFfT1BUSU9OX0NVUlJFTlQJImN1cnJlbnQiCiAKIGludCBkZWJ1 ZywgZnN0YWJfc3R5bGUsIHZlcmJvc2U7CitzdGF0aWMgY2hhciAqKm1udF9hcmd2Oworc3Rh dGljIGludCBtbnRfYXJndl9zaXplOworc3RhdGljIGludCBtbnRfYXJnYzsKIAotc3RydWN0 IGNwYSB7Ci0JY2hhcgkqKmE7Ci0JaW50CWM7Ci19OwotCiBjaGFyICAgKmNhdG9wdChjaGFy ICosIGNvbnN0IGNoYXIgKik7CiBzdHJ1Y3Qgc3RhdGZzICpnZXRtbnRwdChjb25zdCBjaGFy ICopOwogaW50CWhhc29wdChjb25zdCBjaGFyICosIGNvbnN0IGNoYXIgKik7CiBpbnQJaXNt b3VudGVkKHN0cnVjdCBmc3RhYiAqLCBzdHJ1Y3Qgc3RhdGZzICosIGludCk7CiBpbnQJaXNy ZW1vdW50YWJsZShjb25zdCBjaGFyICopOwotdm9pZAltYW5nbGUoY2hhciAqLCBzdHJ1Y3Qg Y3BhICopOworc3RhdGljIHZvaWQJbWFuZ2xlKGNoYXIgKik7CiBjaGFyICAgKnVwZGF0ZV9v cHRpb25zKGNoYXIgKiwgY2hhciAqLCBpbnQpOwogaW50CW1vdW50ZnMoY29uc3QgY2hhciAq LCBjb25zdCBjaGFyICosIGNvbnN0IGNoYXIgKiwKIAkJCWludCwgY29uc3QgY2hhciAqLCBj b25zdCBjaGFyICopOwpAQCAtNTAxLDI4ICs0OTksMjQgQEAKIH0KIAogc3RhdGljIHZvaWQK LWFwcGVuZF9hcmcoc3RydWN0IGNwYSAqc2EsIGNoYXIgKmFyZykKK2FwcGVuZF9hcmd2KGNo YXIgKmFyZykKIHsKLQlzdGF0aWMgaW50IGFfc3o7Ci0KLQlpZiAoc2EtPmMgKyAxID09IGFf c3opIHsKLQkJYV9zeiA9IGFfc3ogPT0gMCA/IDggOiBhX3N6ICogMjsKLQkJc2EtPmEgPSBy ZWFsbG9jKHNhLT5hLCBzaXplb2Yoc2EtPmEpICogYV9zeik7Ci0JCWlmIChzYS0+YSA9PSBO VUxMKQorCWlmIChtbnRfYXJnYyA9PSBtbnRfYXJndl9zaXplKSB7CisJCW1udF9hcmd2X3Np emUgPSBtbnRfYXJndl9zaXplID09IDAgPyAxNiA6IG1udF9hcmd2X3NpemUgKiAyOworCQlt bnRfYXJndiA9IHJlYWxsb2MobW50X2FyZ3YsIHNpemVvZigqbW50X2FyZ3YpICogbW50X2Fy Z3Zfc2l6ZSk7CisJCWlmIChtbnRfYXJndiA9PSBOVUxMKQogCQkJZXJyeCgxLCAicmVhbGxv YyBmYWlsZWQiKTsKIAl9Ci0Jc2EtPmFbKytzYS0+Y10gPSBhcmc7CisJbW50X2FyZ3ZbbW50 X2FyZ2MrK10gPSBhcmc7CiB9CiAKIGludAogbW91bnRmcyhjb25zdCBjaGFyICp2ZnN0eXBl LCBjb25zdCBjaGFyICpzcGVjLCBjb25zdCBjaGFyICpuYW1lLCBpbnQgZmxhZ3MsCiAJY29u c3QgY2hhciAqb3B0aW9ucywgY29uc3QgY2hhciAqbW50b3B0cykKIHsKLQlzdHJ1Y3QgY3Bh IG1udF9hcmd2OwogCXN0cnVjdCBzdGF0ZnMgc2Y7CiAJaW50IGksIHJldDsKIAljaGFyICpv cHRidWYsIGV4ZWNuYW1lW1BBVEhfTUFYXSwgbW50cGF0aFtQQVRIX01BWF07Ci0Jc3RhdGlj IGludCBtbnRfYXJndl9pbml0ZWQ7CiAKIAkvKiByZXNvbHZlIHRoZSBtb3VudHBvaW50IHdp dGggcmVhbHBhdGgoMykgKi8KIAkodm9pZCljaGVja3BhdGgobmFtZSwgbW50cGF0aCk7CkBA IC01NTcsMzIgKzU1MSwyOSBAQAogCS8qIENvbnN0cnVjdCB0aGUgbmFtZSBvZiB0aGUgYXBw cm9wcmlhdGUgbW91bnQgY29tbWFuZCAqLwogCSh2b2lkKXNucHJpbnRmKGV4ZWNuYW1lLCBz aXplb2YoZXhlY25hbWUpLCAibW91bnRfJXMiLCB2ZnN0eXBlKTsKIAotCWlmICghbW50X2Fy Z3ZfaW5pdGVkKSB7Ci0JCW1udF9hcmd2X2luaXRlZCsrOwotCQltbnRfYXJndi5hID0gTlVM TDsKLQl9Ci0JbW50X2FyZ3YuYyA9IC0xOwotCWFwcGVuZF9hcmcoJm1udF9hcmd2LCBleGVj bmFtZSk7Ci0JbWFuZ2xlKG9wdGJ1ZiwgJm1udF9hcmd2KTsKLQlhcHBlbmRfYXJnKCZtbnRf YXJndiwgc3RyZHVwKHNwZWMpKTsKLQlhcHBlbmRfYXJnKCZtbnRfYXJndiwgc3RyZHVwKG5h bWUpKTsKLQlhcHBlbmRfYXJnKCZtbnRfYXJndiwgTlVMTCk7CisJbW50X2FyZ2MgPSAwOyAv KiBSZXNldCBhcmd1bWVudCB2ZWN0b3IgKi8KKwlhcHBlbmRfYXJndihleGVjbmFtZSk7CisJ bWFuZ2xlKG9wdGJ1Zik7CisJYXBwZW5kX2FyZ3Yoc3RyZHVwKHNwZWMpKTsKKwlhcHBlbmRf YXJndihzdHJkdXAobmFtZSkpOworCWFwcGVuZF9hcmd2KE5VTEwpOworCS0tbW50X2FyZ2M7 IC8qIERvIG5vdCBjb3VudCB0aGUgdGVybWluYXRpbmcgbnVsbCBwb2ludGVyICovCiAKIAlp ZiAoZGVidWcpIHsKIAkJaWYgKHVzZV9tb3VudHByb2codmZzdHlwZSkpCiAJCQlwcmludGYo ImV4ZWM6IG1vdW50XyVzIiwgdmZzdHlwZSk7CiAJCWVsc2UKIAkJCXByaW50ZigibW91bnQg LXQgJXMiLCB2ZnN0eXBlKTsKLQkJZm9yIChpID0gMTsgaSA8IG1udF9hcmd2LmM7IGkrKykK LQkJCSh2b2lkKXByaW50ZigiICVzIiwgbW50X2FyZ3YuYVtpXSk7CisJCWZvciAoaSA9IDE7 IGkgPCBtbnRfYXJnYzsgaSsrKQorCQkJKHZvaWQpcHJpbnRmKCIgJXMiLCBtbnRfYXJndltp XSk7CiAJCSh2b2lkKXByaW50ZigiXG4iKTsKIAkJcmV0dXJuICgwKTsKIAl9CiAKIAlpZiAo dXNlX21vdW50cHJvZyh2ZnN0eXBlKSkgewotCQlyZXQgPSBleGVjX21vdW50cHJvZyhuYW1l LCBleGVjbmFtZSwgbW50X2FyZ3YuYSk7CisJCXJldCA9IGV4ZWNfbW91bnRwcm9nKG5hbWUs IGV4ZWNuYW1lLCBtbnRfYXJndik7CiAJfSBlbHNlIHsKLQkJcmV0ID0gbW91bnRfZnModmZz dHlwZSwgbW50X2FyZ3YuYywgbW50X2FyZ3YuYSk7CisJCXJldCA9IG1vdW50X2ZzKHZmc3R5 cGUsIG1udF9hcmdjLCBtbnRfYXJndik7CiAJfQogCiAJZnJlZShvcHRidWYpOwpAQCAtNjg0 LDggKzY3NSw4IEBACiAJcmV0dXJuIChjcCk7CiB9CiAKLXZvaWQKLW1hbmdsZShjaGFyICpv cHRpb25zLCBzdHJ1Y3QgY3BhICphKQorc3RhdGljIHZvaWQKK21hbmdsZShjaGFyICpvcHRp b25zKQogewogCWNoYXIgKnAsICpzOwogCkBAIC03MjAsMTUgKzcxMSwxNSBAQAogCQkJICAg IHNpemVvZihncm91cHF1b3RhZXEpIC0gMSkgPT0gMCkgewogCQkJCWNvbnRpbnVlOwogCQkJ fSBlbHNlIGlmICgqcCA9PSAnLScpIHsKLQkJCQlhcHBlbmRfYXJnKGEsIHApOworCQkJCWFw cGVuZF9hcmd2KHApOwogCQkJCXAgPSBzdHJjaHIocCwgJz0nKTsKIAkJCQlpZiAocCAhPSBO VUxMKSB7CiAJCQkJCSpwID0gJ1wwJzsKLQkJCQkJYXBwZW5kX2FyZyhhLCBwICsgMSk7CisJ CQkJCWFwcGVuZF9hcmd2KHAgKyAxKTsKIAkJCQl9CiAJCQl9IGVsc2UgewotCQkJCWFwcGVu ZF9hcmcoYSwgc3RyZHVwKCItbyIpKTsKLQkJCQlhcHBlbmRfYXJnKGEsIHApOworCQkJCWFw cGVuZF9hcmd2KHN0cmR1cCgiLW8iKSk7CisJCQkJYXBwZW5kX2FyZ3YocCk7CiAJCQl9CiAJ CX0KIH0K --------------050908040806000001080607--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?496B10F9.20201>