From owner-freebsd-bugs@FreeBSD.ORG Thu Jan 7 17:20:05 2010 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id F3F221065672 for ; Thu, 7 Jan 2010 17:20:04 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id C9D678FC13 for ; Thu, 7 Jan 2010 17:20:04 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.3/8.14.3) with ESMTP id o07HK4kP087525 for ; Thu, 7 Jan 2010 17:20:04 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.3/8.14.3/Submit) id o07HK4W8087524; Thu, 7 Jan 2010 17:20:04 GMT (envelope-from gnats) Date: Thu, 7 Jan 2010 17:20:04 GMT Message-Id: <201001071720.o07HK4W8087524@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org From: Efstratios Karatzas Cc: Subject: Re: bin/139606: pkg_add(1) coredumps silently on atlantis symlink X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Efstratios Karatzas List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Jan 2010 17:20:05 -0000 The following reply was made to PR bin/139606; it has been noted by GNATS. From: Efstratios Karatzas To: bug-followup@freebsd.org, phk@critter.freebsd.dk Cc: Subject: Re: bin/139606: pkg_add(1) coredumps silently on atlantis symlink Date: Thu, 7 Jan 2010 19:13:06 +0200 --0016e6d784ee4cb473047c962fe0 Content-Type: text/plain; charset=UTF-8 Hello! Why pkg_add crashes: The problem exists in function fexists() which resides in file src/usr.sbin/pkg_install/lib/file.c The function is supposed to check if a file exists, but lstat(2) is being used instead of stat(2). lstat(2) checks only if the symbolic link file exists and not the actual file that the symbolic link points to. So, the symbolic link file exists, lstat returns 0. In src/usr.sbin/pkg_install/add/main.c we pass the check in line #247 and strdup(3) crashes because realpath(3) returns NULL. realpath() returns NULL because the actual file does not really exist. Fix: Instead of lstat(2) in file.c use stat(2). There is no reason to use lstat since we don't want to perform any special checks in case it is a sym link. So we use stat(2) which checks if the actual file that the symlink points to exists. This is done in patch-a-1.diff But the strdup(realpath()) call is still likely to cause a seg fault because there is a race condition: Check done (ok) -> file erased somehow -> realpath returns NULL -> strdup goes boom. So we check for the return value of the realpath() function too. This is done in patch-b-1.diff With these patches, the utility will just exist gracefully with an appropriate error message when, in pkg_perform, it cannot stat the actual file, so no crashes: ps: A lot of race conditions still exist in this utility but that is for another pr, another time. don't know if these are of any use but the original files I used are: main.c SVN rev 201226 file.c SVN rev 198460 Cheers -- Efstratios "GPF" Karatzas --0016e6d784ee4cb473047c962fe0 Content-Type: application/octet-stream; name="patch-a-1.diff" Content-Disposition: attachment; filename="patch-a-1.diff" Content-Transfer-Encoding: base64 X-Attachment-Id: f_g45ryqr81 LS0tIGZpbGUub3JpZy5jCTIwMTAtMDEtMDcgMTc6NTg6MTkuMDAwMDAwMDAwICswMjAwCisrKyBm aWxlLmMJMjAxMC0wMS0wNyAxNzo1OToyNy4wMDAwMDAwMDAgKzAyMDAKQEAgLTMyLDcgKzMyLDcg QEAKIGZleGlzdHMoY29uc3QgY2hhciAqZm5hbWUpCiB7CiAgICAgc3RydWN0IHN0YXQgZHVtbXk7 Ci0gICAgaWYgKCFsc3RhdChmbmFtZSwgJmR1bW15KSkKKyAgICBpZiAoIXN0YXQoZm5hbWUsICZk dW1teSkpCiAJcmV0dXJuIFRSVUU7CiAgICAgcmV0dXJuIEZBTFNFOwogfQo= --0016e6d784ee4cb473047c962fe0 Content-Type: application/octet-stream; name="patch-b-1.diff" Content-Disposition: attachment; filename="patch-b-1.diff" Content-Transfer-Encoding: base64 X-Attachment-Id: f_g45rz8tm2 LS0tIG1haW4ub3JpZy5jCTIwMTAtMDEtMDcgMTg6Mjg6NTguMDAwMDAwMDAwICswMjAwCisrKyBt YWluLmMJMjAxMC0wMS0wNyAxODozNjoxNC4wMDAwMDAwMDAgKzAyMDAKQEAgLTI0NCw4ICsyNDQs MTIgQEAKIAkJICAgIGVycngoMSwgInBhY2thZ2UgbmFtZSB0b28gbG9uZyIpOwogCQlwa2dzW2No XSA9IHN0cmR1cCh0ZW1wKTsKIAkgICAgfSBlbHNlIHsJCQkvKiBleHBhbmQgYWxsIHBhdGhuYW1l cyB0byBmdWxsbmFtZXMgKi8KLQkJaWYgKGZleGlzdHMoKmFyZ3YpKSAvKiByZWZlcnMgdG8gYSBm aWxlIGRpcmVjdGx5ICovCi0JCSAgICBwa2dzW2NoXSA9IHN0cmR1cChyZWFscGF0aCgqYXJndiwg dGVtcCkpOworCQlpZiAoZmV4aXN0cygqYXJndikpIHsgLyogcmVmZXJzIHRvIGEgZmlsZSBkaXJl Y3RseSAqLworCQkgICAgLyogcmFjZSBjb25kaXRpb24gc28gY2hlY2sgZm9yIHJldHVybiB2YWx1 ZSBvZiByZWFscGF0aCgpICovCisJCSAgICBpZiAocmVhbHBhdGgoKmFyZ3YsIHRlbXApKSB7CisJ CQlwa2dzW2NoXSA9IHN0cmR1cCh0ZW1wKTsKKwkJICAgIH0KKwkJfQogCQllbHNlIHsJCS8qIGxv b2sgZm9yIHRoZSBmaWxlIGluIHRoZSBleHBlY3RlZCBwbGFjZXMgKi8KIAkJICAgIGlmICghKGNw ID0gZmlsZUZpbmRCeVBhdGgoTlVMTCwgKmFyZ3YpKSkgewogCQkJLyogbGV0IHBrZ19kbygpIGZh aWwgbGF0ZXIsIHNvIHRoYXQgZXJyb3IgaXMgcmVwb3J0ZWQgKi8K --0016e6d784ee4cb473047c962fe0--