From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io1-xd44.google.com (mail-io1-xd44.google.com [IPv6:2607:f8b0:4864:20::d44]) by sourceware.org (Postfix) with ESMTPS id 71BF03857004 for ; Fri, 17 Jul 2020 22:32:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 71BF03857004 Received: by mail-io1-xd44.google.com with SMTP id i4so11977483iov.11 for ; Fri, 17 Jul 2020 15:32:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=Rb5v2RIFkYcBmr15bomPQEE3nAoBddNbaYP7HssObJs=; b=hlFpXG0CX0Yf9mJ+vm/zWOVfl1cP6VhAXoveVzDLAEQMiyQVIBVcjzRPQjDoOpC5SE yy3flzMDZ5mpWiG1BO+3uTuj6Ag8WkjwqFqptuiHieVHxQD8VtftsAR8hZExGD739GnC tQgjo6QI4T7iuaJEBWvHptnyrlm0s7UI3mcun7lsooNnV9diMwk6e6Lol/RT/Nhr5tLk WnP0wu/k3BJ3kpAWZCMT/RR++Mwg2CuSna8yiEMqS4vT7uf1jz203AQjI6uhvrV9v+gm ERdD+9krth+d9GH+m3ueDYtM7aKEowz+p1GxmkKWROftNplk0mkWn28Zow8l+i9RUn7N NumQ== X-Gm-Message-State: AOAM533+gUWLXSjfAySUyt73yfX+gSFsJCvEbKOTLPriegZ8cvMWL6KD pnSZcFda2HfGpsE2A/UThUOwmQ1aYW5YZ2fFLvw= X-Google-Smtp-Source: ABdhPJzacMVtI9w67nY1hpw7uXzyoDFO2+0O82nwXWjLtw+piFULWbqBSCDjjdMYfB7KAJNDcfDHeiD+F3opf74V+4E= X-Received: by 2002:a02:5b83:: with SMTP id g125mr13219959jab.91.1595025129499; Fri, 17 Jul 2020 15:32:09 -0700 (PDT) MIME-Version: 1.0 References: <20200716112651.2257283-1-hjl.tools@gmail.com> <87o8ofy8e7.fsf@oldenburg2.str.redhat.com> <56cafa21-37ea-b39e-8c84-afb258f0d17a@redhat.com> <20200717222735.GD8268@aurel32.net> In-Reply-To: <20200717222735.GD8268@aurel32.net> From: "H.J. Lu" Date: Fri, 17 Jul 2020 15:31:33 -0700 Message-ID: Subject: Re: V2 [PATCH] nptl: Zero-extend arguments to SETXID syscalls [BZ #26248] To: "Carlos O'Donell" , Florian Weimer , "H.J. Lu via Libc-alpha" Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 Jul 2020 22:32:12 -0000 On Fri, Jul 17, 2020 at 3:27 PM Aurelien Jarno wrote: > > On 2020-07-17 12:42, H.J. Lu via Libc-alpha wrote: > > On Fri, Jul 17, 2020 at 8:02 AM Carlos O'Donell wrote: > > > > > > On 7/16/20 8:46 AM, H.J. Lu via Libc-alpha wrote: > > > > On Thu, Jul 16, 2020 at 5:03 AM Florian Weimer wrote: > > > >> > > > >> * H. J. Lu via Libc-alpha: > > > >> > > > >>> nptl has > > > >>> > > > >>> /* Opcodes and data types for communication with the signal handler to > > > >>> change user/group IDs. */ > > > >>> struct xid_command > > > >>> { > > > >>> int syscall_no; > > > >>> long int id[3]; > > > >>> volatile int cntr; > > > >>> volatile int error; > > > >>> }; > > > >>> > > > >>> /* This must be last, otherwise the current thread might not have > > > >>> permissions to send SIGSETXID syscall to the other threads. */ > > > >>> result = INTERNAL_SYSCALL_NCS (cmdp->syscall_no, 3, > > > >>> cmdp->id[0], cmdp->id[1], cmdp->id[2]); > > > >>> > > > >>> But the second argument of setgroups syscal is a pointer: > > > >>> > > > >>> int setgroups(size_t size, const gid_t *list); > > > >>> > > > >>> But on x32, pointers passed to syscall must have pointer type so that they > > > >>> will be zero-extended. > > > >>> > > > >>> Add to define INTERNAL_SETXID_SYSCALL_NCS and use it, > > > >>> instead of INTERNAL_SYSCALL_NCS, for SETXID syscalls. X32 override it > > > >>> with pointer type for setgroups. A testcase is added and setgroups > > > >>> returned with EFAULT when running as root without the fix. > > > >> > > > >> Isn't it sufficient to change the type of id to unsigned long int[3]? > > > >> The UID arguments are unsigned on the kernel side, so no sign extension > > > >> is required. > > > >> > > > > > > > > It works. Here is the updated patch. OK for master? > > > > > > > > Thanks. > > > > > > > > > > This test should run in a container, and it should attempt two setgroups > > > calls, one with groups and one empty with a bad address. > > > > Fixed. > > > > > > From 2af9e56c2306dc9d80a4476fa5b154a26a935557 Mon Sep 17 00:00:00 2001 > > > > From: "H.J. Lu" > > > > Date: Thu, 16 Jul 2020 03:37:10 -0700 > > > > Subject: [PATCH] nptl: Zero-extend arguments to SETXID syscalls [BZ #26248] > > > > > > > > nptl has > > > > > > > > /* Opcodes and data types for communication with the signal handler to > > > > change user/group IDs. */ > > > > struct xid_command > > > > { > > > > int syscall_no; > > > > long int id[3]; > > > > volatile int cntr; > > > > volatile int error; > > > > }; > > > > > > > > /* This must be last, otherwise the current thread might not have > > > > permissions to send SIGSETXID syscall to the other threads. */ > > > > result = INTERNAL_SYSCALL_NCS (cmdp->syscall_no, 3, > > > > cmdp->id[0], cmdp->id[1], cmdp->id[2]); > > > > > > > > But the second argument of setgroups syscal is a pointer: > > > > > > > > int setgroups(size_t size, const gid_t *list); > > > > > > > > But on x32, pointers passed to syscall must have pointer type so that they > > > > will be zero-extended. Since the XID arguments are unsigned on the kernel > > > > side, so no sign extension is required. Change xid_command to > > > > > > > > struct xid_command > > > > { > > > > int syscall_no; > > > > unsigned long int id[3]; > > > > volatile int cntr; > > > > volatile int error; > > > > }; > > > > > > > > so that all arguments zero-extended. A testcase is added for x32 and > > > > setgroups returned with EFAULT when running as root without the fix. > > > > --- > > > > nptl/descr.h | 8 ++- > > > > sysdeps/unix/sysv/linux/x86_64/x32/Makefile | 4 ++ > > > > .../sysv/linux/x86_64/x32/tst-setgroups.c | 67 +++++++++++++++++++ > > > > 3 files changed, 78 insertions(+), 1 deletion(-) > > > > create mode 100644 sysdeps/unix/sysv/linux/x86_64/x32/tst-setgroups.c > > Is there a reason to limit that test to x32? We do not have any other > getgroups or setgroups test in the GLIBC tree, so that simple test might > already be able to catch issues. In addition such a test would benefit > the other ILP32 architectures supported by GLIBC. > I can move it to nptl. -- H.J.