From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi1-x236.google.com (mail-oi1-x236.google.com [IPv6:2607:f8b0:4864:20::236]) by sourceware.org (Postfix) with ESMTPS id 22A113858D32 for ; Tue, 26 Jul 2022 16:51:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 22A113858D32 Received: by mail-oi1-x236.google.com with SMTP id s204so17754048oif.5 for ; Tue, 26 Jul 2022 09:51:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=wxzv/E+UaR2pmMOlye+KVg27MhKJv4mjPxrciwoZWQs=; b=Q85wvLg6RAnMUS748QTGAYEoN66q1oh4XIGEUGR8Ri2ies9t3lLeFrXA2XKBEydjRV n2nne7aUdfA9joiYFIqbN2yFusPGbAG86Xgqxv0pOujS176Nop+NnTMpVBq1hHcSnPoU oGFaasvLUorEGBxrVoEmqcjd+2BKu9xRBAGSkcbJPv0w7+w07L9fJ7vGk3dqJ11GrmUf 3OMGTpfjXev0AuB+WVcXmInfYSMjoz4NbpspWgVnXv/mdo4MwbDVhGcfadQxAuRiZ4oE 3e9F8C+QCem1V7mBX7fTXmLzx7SDPc/GcUsRsg7Z9IddgRy2cdpFylDhHw4lwgtyFrgh +aMw== X-Gm-Message-State: AJIora++cxvEyh4PjRCH6PTMq3I/ZZ+M8NpGLqNio0wXWuiI7AqEDtxZ dZntRuF3kxgH3eQZKWD3RzFO5nEFiRZwYIRp85nPx5yp X-Google-Smtp-Source: AGRyM1vsuJ1rPiAGi4kZGlsA7p5B2rHWEPTLlj2r4WK04VfTO9wTQfARPgny53bX/mPo/7HGlwQHqTHjh/lpBtQgHlU= X-Received: by 2002:aca:3946:0:b0:33a:7585:494b with SMTP id g67-20020aca3946000000b0033a7585494bmr69073oia.164.1658854274546; Tue, 26 Jul 2022 09:51:14 -0700 (PDT) MIME-Version: 1.0 References: <20220725225728.824128-1-Jason@zx2c4.com> <20220725232810.843433-1-Jason@zx2c4.com> In-Reply-To: From: Mark Harris Date: Tue, 26 Jul 2022 09:51:03 -0700 Message-ID: Subject: Re: [PATCH v2] arc4random: simplify design for better safety To: "Jason A. Donenfeld" Cc: libc-alpha@sourceware.org, Florian Weimer , linux-crypto@vger.kernel.org Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-2.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.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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: Tue, 26 Jul 2022 16:51:16 -0000 Jason A. Donenfeld wrote: > On Mon, Jul 25, 2022 at 06:10:06PM -0700, Mark Harris wrote: > > Jason A. Donenfeld wrote: > > > + l = __getrandom_nocancel (p, n, 0); > > > + if (l > 0) > > > + { > > > + if ((size_t) l == n) > > > + return; /* Done reading, success. */ > > > + p = (uint8_t *) p + l; > > > + n -= l; > > > + continue; /* Interrupted by a signal; keep going. */ > > > + } > > > + else if (l == 0) > > > + arc4random_getrandom_failure (); /* Weird, should never happen. */ > > > + else if (errno == ENOSYS) > > > + { > > > + have_getrandom = false; > > > + break; /* No syscall, so fallback to /dev/urandom. */ > > > + } > > > + arc4random_getrandom_failure (); /* Unknown error, should never happen. */ > > > > Isn't EINTR also possible? Aborting in that case does not seem reasonable. > > Not in current kernels, where it always returns at least PAGE_SIZE bytes > before checking for pending signals. In older kernels, if there was a > signal pending at the top, it would do no work and return -ERESTARTSYS, > which I believe should then get restarted by glibc's syscaller? I might > be wrong about how restarts work though, so if you know better, please > let me know. TEMP_FAILURE_RETRY relies on errno, so that's not what we > want. I guess I can just add a case for it. > > > Also the __getrandom_nocancel function does not set errno on Linux; it > > just returns INTERNAL_SYSCALL_CALL (getrandom, buf, buflen, flags). > > So unless that is changed, it doesn't look like this ENOSYS check will > > detect old Linux kernels. > > Thanks. It looks like INTERNAL_SYSCALL_CALL just returns the errno as-is > as a return value, right? I'll adjust the code to account for that. Yes INTERNAL_SYSCALL_CALL just returns the negated errno value that it gets from the Linux kernel, but only on Linux does __getrandom_nocancel use that. The Hurd and generic implementations set errno on error. Previously the only call to this function did not care about the specific error value so it didn't matter. Since you are now using the error value in generic code, __getrandom_nocancel should be changed on Linux to set errno like most other _nocancel calls, and then it should go back to checking errno here. And as Adhemerval mentioned, you only added a Linux implementation of __ppoll_infinity_nocancel, but are calling it from generic code. Also, by the way your patches cc'd directly to me get quarantined because DKIM signature verification failed. The non-patch messages pass DKIM and are fine. - Mark