From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752442AbcBVQ53 (ORCPT ); Mon, 22 Feb 2016 11:57:29 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55462 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751937AbcBVQ51 (ORCPT ); Mon, 22 Feb 2016 11:57:27 -0500 Subject: Re: [PATCH] assoc_array: don't call compare_object() on a node To: David Howells References: <1456157620-20819-1-git-send-email-jmarchan@redhat.com> <12613.1456159058@warthog.procyon.org.uk> Cc: keyrings@vger.kernel.org, linux-kernel@vger.kernel.org From: Jerome Marchand X-Enigmail-Draft-Status: N1110 Message-ID: <56CB3DF0.9020805@redhat.com> Date: Mon, 22 Feb 2016 17:57:20 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <12613.1456159058@warthog.procyon.org.uk> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="EbqWnawiPjSdaetfuwTsJG94fMuSD04OF" X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Mon, 22 Feb 2016 16:57:27 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --EbqWnawiPjSdaetfuwTsJG94fMuSD04OF Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 02/22/2016 05:37 PM, David Howells wrote: > Jerome Marchand wrote: >=20 >> In assoc_array_insert_into_terminal_node(), we call the >> compare_object() method on all empty slots, Sorry, this is a typo. It should be "on all non-empty slots". >=20 > Ummm... That shouldn't happen - the: >=20 > if (!ptr) { > free_slot =3D i; > continue; > } >=20 > preceding the line you modified should cause the comparison to be skipp= ed on a > slot if it's empty. >=20 >> even when they're not leaves, passing a pointer to an unexpected struc= ture >> to compare_object(). >=20 > Do you instead mean a metadata pointer rather than an empty slot? Yes. In the cases I debugged, it was a node, but I guess we could encounter a shortcut here too. >=20 >> Currently it causes an out-of-bound read access in keyring_compare_obj= ect >> detected by KASan. The issue is easily reproduced with keyutils tests= uite. >=20 > I don't see it. Did you modify the testsuite, or is it a matter of run= ning it > often enough? Do you have KASan enabled? In my experience, the reproduction is systematic on some test (e.g. keyctl/unlink/all). AFAIK, the testsuite isn't modify (it's from Red Hat test infrastructure). >=20 > Also, can you include the oops output you get in the patch description,= > please? Sure. >=20 > That said, I can see that there is probably an issue that your patch fi= xes - > but it's not quite the one you describe (see above). Does the description sounds correct if you add the missing negation? Jerome >=20 > David >=20 --EbqWnawiPjSdaetfuwTsJG94fMuSD04OF Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJWyz30AAoJEHTzHJCtsuoCtjEH/j1av8mySDq21RiIUIVrXq+H uy779keDqhu8VL/iB/qbHC9xsfMealpzr7hkxavLX5enUywIhJzVkRx+4yHK6rES lVBDL+bjdeeONTIHEclZmxbuKZii7Q6R2a1eot+Z7G13ilWqRlnrGGw9ftAsWufs /2ijveKKpEZDzOdiOz49rvBiPk+ZHT24K5+Gz7SoAJ+0kAxpzN/0ILbvr2VU+kFv aIfwf7XZtoUSMlI4wm4LdGCTskAdgPL8mAON5prO4GSQ4Lj1j89Gp6h9ap318jLR MOTPlCAZOEhkoar/tSvUjDGNNvkrzwL+KlHeikVUmNPSXA0ESM2hlVRj7IFT7E8= =M5Xd -----END PGP SIGNATURE----- --EbqWnawiPjSdaetfuwTsJG94fMuSD04OF--