From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757064AbbEETRz (ORCPT ); Tue, 5 May 2015 15:17:55 -0400 Received: from out02.mta.xmission.com ([166.70.13.232]:42053 "EHLO out02.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751079AbbEETRx (ORCPT ); Tue, 5 May 2015 15:17:53 -0400 From: ebiederm@xmission.com (Eric W. Biederman) To: Nicolas Iooss Cc: Alexander Viro , linux-fsdevel@vger.kernel.org, Andrew Morton , linux-kernel@vger.kernel.org, Linux Containers References: <1429181404.2850.44.camel@perches.com> <1430649246-32726-1-git-send-email-nicolas.iooss_linux@m4x.org> Date: Tue, 05 May 2015 14:13:24 -0500 In-Reply-To: <1430649246-32726-1-git-send-email-nicolas.iooss_linux@m4x.org> (Nicolas Iooss's message of "Sun, 3 May 2015 18:34:05 +0800") Message-ID: <87bnhyhm7f.fsf@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-AID: U2FsdGVkX1+Z/1iP9Bl23mKdUmn1GQgFcbUhosmT/R0= X-SA-Exim-Connect-IP: 67.3.205.90 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.7 XMSubLong Long Subject * 1.5 XMNoVowels Alpha-numberic number with no vowels * 0.0 TVD_RCVD_IP Message was received from an IP address * 0.0 T_TM2_M_HEADER_IN_MSG BODY: No description available. * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa04 1397; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_01 4+ unique symbols in subject * 0.5 XM_Body_Dirty_Words Contains a dirty word X-Spam-DCC: XMission; sa04 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: **;Nicolas Iooss X-Spam-Relay-Country: X-Spam-Timing: total 345 ms - load_scoreonly_sql: 0.05 (0.0%), signal_user_changed: 4.3 (1.2%), b_tie_ro: 3.1 (0.9%), parse: 1.30 (0.4%), extract_message_metadata: 14 (4.2%), get_uri_detail_list: 2.0 (0.6%), tests_pri_-1000: 4.5 (1.3%), tests_pri_-950: 1.27 (0.4%), tests_pri_-900: 1.01 (0.3%), tests_pri_-400: 27 (7.8%), check_bayes: 25 (7.3%), b_tokenize: 7 (1.9%), b_tok_get_all: 8 (2.2%), b_comp_prob: 3.3 (1.0%), b_tok_touch_all: 4.5 (1.3%), b_finish: 1.80 (0.5%), tests_pri_0: 283 (82.1%), tests_pri_500: 4.4 (1.3%), rewrite_mail: 0.00 (0.0%) Subject: Re: [PATCH 1/2] coredump: use from_kuid/kgid_munged when formatting corename X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Wed, 24 Sep 2014 11:00:52 -0600) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Nicolas Iooss writes: > When adding __printf attribute to cn_printf, gcc reports some issues: > > fs/coredump.c:213:5: warning: format '%d' expects argument of type > 'int', but argument 3 has type 'kuid_t' [-Wformat=] > err = cn_printf(cn, "%d", cred->uid); > ^ > fs/coredump.c:217:5: warning: format '%d' expects argument of type > 'int', but argument 3 has type 'kgid_t' [-Wformat=] > err = cn_printf(cn, "%d", cred->gid); > ^ > > These warnings come from the fact that the value of uid/gid needs to be > extracted from the kuid_t/kgid_t structure before being used as an > integer. More precisely, cred->uid and cred->gid need to be converted > to either user-namespace uid/gid or to init_user_ns uid/gid. > > As Documentation/sysctl/kernel.txt does not specify which user namespace > is used to translate %u and %g in core_pattern, but lowercase %p and %i > are used to format pid/tid in the current process namespace, it seems > intuitive that lowercase %u and %g use the current user namespace. I love the logic of lower vs upper case letters in the selection of how an identifier should be used. Unfortunately I can't support it. Converting to anything other than init_user_ns will actually be an ABI break. Which in practice should trump everything else. Further only the global root user can set this value, which largely implies that the program setting the core dump patter will expect the values to be in the initial user namespace. In practice your patch allows any user to put any uid they want on core files which seems to make the uid parameter useless. So for all of the reasons above I think we need to print uids and gids in the initial user namespace unless explicitly requested to do so. > While at it, format uid and gid values with %u instead of %d because > uid_t/__kernel_uid32_t and gid_t/__kernel_gid32_t are unsigned int. > > Signed-off-by: Nicolas Iooss > --- > fs/coredump.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/fs/coredump.c b/fs/coredump.c > index bbbe139ab280..99c8af640c5a 100644 > --- a/fs/coredump.c > +++ b/fs/coredump.c > @@ -209,11 +209,15 @@ static int format_corename(struct core_name *cn, struct coredump_params *cprm) > break; > /* uid */ > case 'u': > - err = cn_printf(cn, "%d", cred->uid); > + err = cn_printf(cn, "%u", > + from_kuid_munged(cred->user_ns, > + cred->uid)); > break; > /* gid */ > case 'g': > - err = cn_printf(cn, "%d", cred->gid); > + err = cn_printf(cn, "%u", > + from_kgid_munged(cred->user_ns, > + cred->gid)); > break; > case 'd': > err = cn_printf(cn, "%d",