From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934536AbcDMLw3 (ORCPT ); Wed, 13 Apr 2016 07:52:29 -0400 Received: from mga09.intel.com ([134.134.136.24]:49930 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934479AbcDMLw1 (ORCPT ); Wed, 13 Apr 2016 07:52:27 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,479,1455004800"; d="scan'208";a="84325794" Message-ID: <1460548402.6620.128.camel@linux.intel.com> Subject: Re: [PATCH v4 2/6] string_helpers: add kstrdup_quotable_cmdline From: Andy Shevchenko To: "Serge E. Hallyn" , Kees Cook Cc: James Morris , Joe Perches , Mimi Zohar , Andrew Morton , Jonathan Corbet , Kalle Valo , Mauro Carvalho Chehab , Guenter Roeck , Jiri Slaby , Paul Moore , Stephen Smalley , Casey Schaufler , Andreas Gruenbacher , Rasmus Villemoes , Ulf Hansson , Vitaly Kuznetsov , linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org Date: Wed, 13 Apr 2016 14:53:22 +0300 In-Reply-To: <20160412211942.GB12324@mail.hallyn.com> References: <1460480085-32263-1-git-send-email-keescook@chromium.org> <1460480085-32263-3-git-send-email-keescook@chromium.org> <20160412211942.GB12324@mail.hallyn.com> Organization: Intel Finland Oy Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.18.5.1-1 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 2016-04-12 at 16:19 -0500, Serge E. Hallyn wrote: > Quoting Kees Cook (keescook@chromium.org): > > > > Provide an escaped (but readable: no inter-argument NULLs) > > commandline > > safe for logging. > > Sorry, have no access to the original mail right now. >> +char *kstrdup_quotable_cmdline(struct task_struct *task, gfp_t gfp) > > +{ > > + char *buffer, *quoted; > > + int i, res; > > + > > + buffer = kmalloc(PAGE_SIZE, GFP_TEMPORARY); > > + if (!buffer) > > + return NULL; > > + > > + res = get_cmdline(task, buffer, PAGE_SIZE - 1); > > + buffer[res] = '\0'; > > + > > + /* Collapse trailing NULLs, leave res pointing to last non- > > NULL. */ > > + while (--res >= 0 && buffer[res] == '\0') > > + ; Nitpick: perhaps leave comment that make more visible /* nothing */ ; (up to you)? > > + > > + /* Replace inter-argument NULLs. */ > > + for (i = 0; i <= res; i++) But why do you need to check = res? It's already checked by previous condition and undoubtfully it's non-'\0'. Forgot to mention this earlier. > > + if (buffer[i] == '\0') > > + buffer[i] = ' '; -- Andy Shevchenko Intel Finland Oy