From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Jones Subject: Re: [PATCH] lib: Always NUL terminate ucs2_as_utf8 Date: Thu, 21 Apr 2016 11:13:44 -0400 Message-ID: <20160421151343.GA3763@redhat.com> References: <1461141427-16361-1-git-send-email-chris@chris-wilson.co.uk> <5717834C.6070802@redhat.com> <20160421121827.GE2829@codeblueprint.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <20160421121827.GE2829@codeblueprint.co.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Matt Fleming Cc: linux-efi@vger.kernel.org, Jason Andryuk , intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, Matthew Garrett , Laszlo Ersek List-Id: linux-efi@vger.kernel.org T24gVGh1LCBBcHIgMjEsIDIwMTYgYXQgMDE6MTg6MjdQTSArMDEwMCwgTWF0dCBGbGVtaW5nIHdy b3RlOgo+ICggR29vZCBMb3JkLCBJIGhhdGUgZG9pbmcgc3RyaW5nIG1hbmlwdWxhdGlvbiBpbiBD ICkKCih5ZXApCgo+IAo+IE9uIFdlZCwgMjAgQXByLCBhdCAwMzoyNTozMlBNLCBMYXN6bG8gRXJz ZWsgd3JvdGU6Cj4gPiAKPiA+IFNvLCAibGVuIiBkb2VzIG5vdCBpbmNsdWRlIHRoZSByb29tIGZv ciB0aGUgdGVybWluYXRpbmcgTlVMLWJ5dGUgaGVyZS4KPiA+IFdoZW4gImxlbiIgaXMgcGFzc2Vk IHRvIHVjczJfYXNfdXRmOCgpLCB3aXRoIHRoZSBwcm9wb3NlZCBwYXRjaCBhcHBsaWVkLAo+ID4g YSBOVUwgYnl0ZSB3aWxsIGJlIHByb2R1Y2VkIGluICJuYW1lIiwgYnV0IGl0IHdpbGwgYmUgYXQg dGhlIHByaWNlIG9mIGEKPiA+IGdlbnVpbmUgY2hhcmFjdGVyIGZyb20gdGhlIGlucHV0IHZhcmlh YmxlIG5hbWUuCj4gCj4gUmlnaHQsIGFuZCB0aGlzIGlzIGEgcHJvYmxlbSBiZWNhdXNlIHdlJ3Jl IHRyeWluZyB0byBrZWVwIHRoZSBuYW1lcwo+IGNvbnNpc3RlbnQgYmV0d2VlbiBlZml2YXJmcyBh bmQgdGhlIEVGSSB2YXJpYWJsZSBkYXRhLiBGb3JjZQo+IE5VTC10ZXJtaW5hdGluZyB0aGUgc3Ry aW5nIGlzIHdyb25nLCBiZWNhdXNlIGlmIHlvdSBoYXZlIG5vIHJvb20gZm9yCj4gdGhlIE5VTCB0 aGUgY2FsbGVyIHNob3VsZCBjaGVjayBmb3IgdGhhdC4gU2FkbHkgbm9uZSBkby4KPiAKPiBPbiB0 aGUgZmxpcC1zaWRlLCBwYXNzaW5nIGFyb3VuZCBub24tTlVMIHRlcm1pbmF0ZWQgc3RyaW5ncyBp cyBqdXN0Cj4gYmVnZ2luZyBmb3IgdGhlc2Uga2luZHMgb2YgaXNzdWVzIHRvIGNvbWUgdXAuCj4g Cj4gVGhlIGZhY3QgaXMgdGhhdCB0aGUgY2FsbGVycyBvZiB1Y3MyX2FzX3V0ZjgoKSBhcmUgcGFz c2luZyBpdCB0aGUKPiB3cm9uZyAnbGVuJyBhcmd1bWVudC4gV2Ugd2FudCBhIE5VTC10ZXJtaW5h dGVkIHV0Zjggc3RyaW5nIGFuZCB3ZSdyZQo+IHBhc3NpbmcgYSBOVUwtdGVybWluYXRlZCB1Y3My IHN0cmluZy4gV2Ugc2hvdWxkIHRlbGwgdWNzMl9hc191dGY4KCkgaXQKPiBoYXMgZW5vdWdoIHJv b20gdG8gY29weSB0aGUgTlVMLgo+IAo+IFdvdWxkbid0IHRoaXMgd29yayAobWludXMgdGhlIHJl dHVybiB2YWx1ZSBjaGVja2luZyk/CgpJIGFncmVlIHdpdGggeW91ciBhbmFseXNpcywgYW5kIHlv dXIgcGF0Y2ggbG9va3MgcGxhdXNpYmxlLgoKLS0gCiAgUGV0ZXIKX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4IG1haWxpbmcgbGlzdApJbnRl bC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3Jn L21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753178AbcDUPNw (ORCPT ); Thu, 21 Apr 2016 11:13:52 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49415 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752059AbcDUPNv (ORCPT ); Thu, 21 Apr 2016 11:13:51 -0400 Date: Thu, 21 Apr 2016 11:13:44 -0400 From: Peter Jones To: Matt Fleming Cc: Laszlo Ersek , Chris Wilson , intel-gfx@lists.freedesktop.org, Jason Andryuk , Matthew Garrett , linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] lib: Always NUL terminate ucs2_as_utf8 Message-ID: <20160421151343.GA3763@redhat.com> References: <1461141427-16361-1-git-send-email-chris@chris-wilson.co.uk> <5717834C.6070802@redhat.com> <20160421121827.GE2829@codeblueprint.co.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20160421121827.GE2829@codeblueprint.co.uk> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Apr 21, 2016 at 01:18:27PM +0100, Matt Fleming wrote: > ( Good Lord, I hate doing string manipulation in C ) (yep) > > On Wed, 20 Apr, at 03:25:32PM, Laszlo Ersek wrote: > > > > So, "len" does not include the room for the terminating NUL-byte here. > > When "len" is passed to ucs2_as_utf8(), with the proposed patch applied, > > a NUL byte will be produced in "name", but it will be at the price of a > > genuine character from the input variable name. > > Right, and this is a problem because we're trying to keep the names > consistent between efivarfs and the EFI variable data. Force > NUL-terminating the string is wrong, because if you have no room for > the NUL the caller should check for that. Sadly none do. > > On the flip-side, passing around non-NUL terminated strings is just > begging for these kinds of issues to come up. > > The fact is that the callers of ucs2_as_utf8() are passing it the > wrong 'len' argument. We want a NUL-terminated utf8 string and we're > passing a NUL-terminated ucs2 string. We should tell ucs2_as_utf8() it > has enough room to copy the NUL. > > Wouldn't this work (minus the return value checking)? I agree with your analysis, and your patch looks plausible. -- Peter