From debbugs-submit-bounces@debbugs.gnu.org Thu Jun 24 10:26:55 2021 Received: (at submit) by debbugs.gnu.org; 24 Jun 2021 14:26:56 +0000 Received: from localhost ([127.0.0.1]:43656 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lwQJj-0003AK-Dh for submit@debbugs.gnu.org; Thu, 24 Jun 2021 10:26:55 -0400 Received: from lists.gnu.org ([209.51.188.17]:39112) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lwQJg-0003A9-2m for submit@debbugs.gnu.org; Thu, 24 Jun 2021 10:26:53 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:57560) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lwQJf-0007dt-Jq for bug-coreutils@gnu.org; Thu, 24 Jun 2021 10:26:51 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:60160) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lwQJW-0002v8-OP for bug-coreutils@gnu.org; Thu, 24 Jun 2021 10:26:48 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1624544799; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=775fx7dKRb0tKbft3CR+TIPOfj8cxExJhCN1BCBZvu0=; b=WhI92jR54PCbZi6KWsVs/6kbF5r4vgYATDJ6qsuGrcNofc+mqv709l5fljff7RLIqs0lhU 47QFEXsg6XUS/Frk5FUMCSwzBod8uhuqCnN2k2wRK6ioF4wxNCnQ3EsJADn9nLyfXtVL1P Il2Oo4EJdiZpnuLXNWxXNatcify8XXw= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-476--JPJOH-3O_6UQZiBZ8hC6w-1; Thu, 24 Jun 2021 10:26:38 -0400 X-MC-Unique: -JPJOH-3O_6UQZiBZ8hC6w-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 376FB84F228 for ; Thu, 24 Jun 2021 14:26:36 +0000 (UTC) Received: from nbkamil.localnet (unknown [10.43.7.71]) by smtp.corp.redhat.com (Postfix) with ESMTP id 022577EBFE; Thu, 24 Jun 2021 14:26:34 +0000 (UTC) From: Kamil Dudka To: bug-coreutils@gnu.org Subject: coreutils: stack out-of-bounds write in tail --follow Date: Thu, 24 Jun 2021 16:26:33 +0200 Message-ID: <1840531.tdWV9SEqCh@nbkamil> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=kdudka@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Received-SPF: pass client-ip=170.10.133.124; envelope-from=kdudka@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -31 X-Spam_score: -3.2 X-Spam_bar: --- X-Spam_report: (-3.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.362, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-Spam-Score: -1.4 (-) X-Debbugs-Envelope-To: submit Cc: sbroz@redhat.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -2.4 (--) Hello, As originally reported by Stepan Broz (CC'd), tail --follow crashes when it is given too many files to follow, and ulimit -n is set to >1024. FD_SET(wd, &rfd) in tail_forever_inotify() writes beyond the stack-allocated variable in case wd >= FD_SETSIZE. Minimal example: # mkdir dir # cd dir # touch {1..1021} # ulimit -n 1025 # tail -f * The out-of-bound write could be fixed like this: --- a/src/tail.c +++ b/src/tail.c @@ -1647,28 +1647,32 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, if (writer_is_dead) exit (EXIT_SUCCESS); writer_is_dead = (kill (pid, 0) != 0 && errno != EPERM); if (writer_is_dead) delay.tv_sec = delay.tv_usec = 0; else { delay.tv_sec = (time_t) sleep_interval; delay.tv_usec = 1000000 * (sleep_interval - delay.tv_sec); } } + if (FD_SETSIZE <= wd) + die (EXIT_FAILURE, 0, + _("too many open files to wait for inotify events")); + fd_set rfd; FD_ZERO (&rfd); FD_SET (wd, &rfd); if (monitor_output) FD_SET (STDOUT_FILENO, &rfd); int file_change = select (MAX (wd, STDOUT_FILENO) + 1, &rfd, NULL, NULL, pid ? &delay: NULL); if (file_change == 0) continue; else if (file_change == -1) die (EXIT_FAILURE, errno, _("error waiting for inotify and output events")); Alternatively, we might rewrite the code to use poll() rather than select(). Kamil From debbugs-submit-bounces@debbugs.gnu.org Thu Jun 24 10:50:34 2021 Received: (at 49209) by debbugs.gnu.org; 24 Jun 2021 14:50:34 +0000 Received: from localhost ([127.0.0.1]:43768 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lwQgc-0003tL-3e for submit@debbugs.gnu.org; Thu, 24 Jun 2021 10:50:34 -0400 Received: from mail-wr1-f44.google.com ([209.85.221.44]:33665) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lwQgb-0003t8-2t for 49209@debbugs.gnu.org; Thu, 24 Jun 2021 10:50:33 -0400 Received: by mail-wr1-f44.google.com with SMTP id d11so7045810wrm.0 for <49209@debbugs.gnu.org>; Thu, 24 Jun 2021 07:50:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=I5UIb9OpxYlXILpHrg+kfIF2E47xn1eFl5wcoREVlRU=; b=lAAc3CMhX784o6A+pKrVv2bS8VX8XmXrAbeqnPrieezit+w7F+Biruy2yA7mDCVqm4 u1C1n+u04SGwmn4th8DkV3V7dqJB59/6V8N4XOadBJaw7k+78O6oVjws/FyDHw9uzGz+ 1t/oWakeyJxtIHiE7Bu5+4CooAysMiaFYS83XAbQeYMQx6DpHLPYnLsqULff8serWs6z 8rOJWNn2aelriV/zRiUWrKTQhHv1251dWIRciniOH+rvIsszN/D2enSMdJD+/A7Z6gA6 h3bSEyG8THP1tStGNr/CqtrfPvRa6y6ORAXv3Du5l1YeynLrV6lxd8ONlXoFhJuw5hhj loYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:subject:to:cc:references:from:message-id :date:user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=I5UIb9OpxYlXILpHrg+kfIF2E47xn1eFl5wcoREVlRU=; b=LpzwcQno+cjeg/y8oh4bpKadz6t+GxCt1hAX7DIUnHf4DL4uu6Lj/20UwRvSbPn7jc wJbY6wYznbnDySWKwf+/iK5deP9746tt9+UO9+D+hQ1HObQMKDtWENO5n4fVmavg804V GPMmnxPGYy1+cW7QcFuKV4RNYl2SVpvFOICJ6yxCmdsG/BB0VbR466ABdJQGTGHgbQPz Z6opNbk5dJfvuN1EpSsR+aGImTtAedlIppPFWKIvYeJOX3P1ktbEsXw9Nqv7zBPBx2ca aino+QnAXqvnzndV5Rvz1mDgnFKlX//6GHcK7uWATAHa1OU8KzBYC+ukt7uQ6pw0duYP U9qQ== X-Gm-Message-State: AOAM531+8ZVtc/w0cYTSSd0q2vS9ao7SqDio2gOelZJFNJMldZGJ7bzj oey2q1hB3oP9Ot7iG7PMQQw= X-Google-Smtp-Source: ABdhPJwVl6Lz8eyG2oAGMJfoiUfh9iQX1kcVGeTqRjpVfQFVnkQykW6kT/i8sPd0knwGsEQuaT70wQ== X-Received: by 2002:a5d:4904:: with SMTP id x4mr4922989wrq.202.1624546227073; Thu, 24 Jun 2021 07:50:27 -0700 (PDT) Received: from localhost.localdomain (86-42-15-3-dynamic.agg2.lod.rsl-rtd.eircom.net. [86.42.15.3]) by smtp.googlemail.com with UTF8SMTPSA id j11sm3084734wms.6.2021.06.24.07.50.26 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 24 Jun 2021 07:50:26 -0700 (PDT) Subject: Re: bug#49209: coreutils: stack out-of-bounds write in tail --follow To: Kamil Dudka , 49209@debbugs.gnu.org References: <1840531.tdWV9SEqCh@nbkamil> From: =?UTF-8?Q?P=c3=a1draig_Brady?= Message-ID: Date: Thu, 24 Jun 2021 15:50:25 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:84.0) Gecko/20100101 Thunderbird/84.0 MIME-Version: 1.0 In-Reply-To: <1840531.tdWV9SEqCh@nbkamil> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Score: 0.5 (/) X-Debbugs-Envelope-To: 49209 Cc: sbroz@redhat.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.5 (/) On 24/06/2021 15:26, Kamil Dudka wrote: > Hello, > > As originally reported by Stepan Broz (CC'd), tail --follow crashes when it > is given too many files to follow, and ulimit -n is set to >1024. > > FD_SET(wd, &rfd) in tail_forever_inotify() writes beyond the stack-allocated > variable in case wd >= FD_SETSIZE. Minimal example: > > # mkdir dir > # cd dir > # touch {1..1021} > # ulimit -n 1025 > # tail -f * > > The out-of-bound write could be fixed like this: > > --- a/src/tail.c > +++ b/src/tail.c > @@ -1647,28 +1647,32 @@ tail_forever_inotify (int wd, struct File_spec *f, > size_t n_files, > if (writer_is_dead) > exit (EXIT_SUCCESS); > > writer_is_dead = (kill (pid, 0) != 0 && errno != EPERM); > > if (writer_is_dead) > delay.tv_sec = delay.tv_usec = 0; > else > { > delay.tv_sec = (time_t) sleep_interval; > delay.tv_usec = 1000000 * (sleep_interval - delay.tv_sec); > } > } > > + if (FD_SETSIZE <= wd) > + die (EXIT_FAILURE, 0, > + _("too many open files to wait for inotify events")); > + > fd_set rfd; > FD_ZERO (&rfd); > FD_SET (wd, &rfd); > if (monitor_output) > FD_SET (STDOUT_FILENO, &rfd); > > int file_change = select (MAX (wd, STDOUT_FILENO) + 1, > &rfd, NULL, NULL, pid ? &delay: NULL); > > if (file_change == 0) > continue; > else if (file_change == -1) > die (EXIT_FAILURE, errno, > _("error waiting for inotify and output events")); > > > Alternatively, we might rewrite the code to use poll() rather than select(). Note the number of descriptors select() is waiting on in independent of the number of files. We should be able to inotify_init() earlier in the process to avoid this issue. I'll have a look. thanks! Pádraig From debbugs-submit-bounces@debbugs.gnu.org Thu Jun 24 11:07:28 2021 Received: (at 49209) by debbugs.gnu.org; 24 Jun 2021 15:07:28 +0000 Received: from localhost ([127.0.0.1]:43829 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lwQwy-0006ax-A9 for submit@debbugs.gnu.org; Thu, 24 Jun 2021 11:07:28 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:48355) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lwQwx-0006ap-7U for 49209@debbugs.gnu.org; Thu, 24 Jun 2021 11:07:27 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1624547246; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=KC+WjAGDTQl5BNXDddZEGPgnk9AWX0Lf+YNQP2gmVaE=; b=DalLs0UEF9h3z/gll5Me4oFwy36GCgQvTJeAyzSmKM/xipkXWmPkPj0yhP4ZLJTsLbwrXR 5WPJQ8LvYDgo7obSgwnOpEy/As4EWIqwucbADKjCeqtql8sTDIzrlj465W09pjur7M43kw g/5rNyTX+WG28ywx6bcVjwNjuJFCpIA= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-30-QcFs_lI_PIqK-yucY8a2mA-1; Thu, 24 Jun 2021 11:07:23 -0400 X-MC-Unique: QcFs_lI_PIqK-yucY8a2mA-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 25093804143; Thu, 24 Jun 2021 15:07:22 +0000 (UTC) Received: from nbkamil.localnet (unknown [10.43.7.71]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6D64560CCC; Thu, 24 Jun 2021 15:07:21 +0000 (UTC) From: Kamil Dudka To: =?ISO-8859-1?Q?P=E1draig?= Brady Subject: Re: bug#49209: coreutils: stack out-of-bounds write in tail --follow Date: Thu, 24 Jun 2021 17:07:20 +0200 Message-ID: <9963052.nUPlyArG6x@nbkamil> In-Reply-To: References: <1840531.tdWV9SEqCh@nbkamil> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=kdudka@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="iso-8859-1" X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 49209 Cc: sbroz@redhat.com, 49209@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) On Thursday, June 24, 2021 4:50:25 PM CEST P=E1draig Brady wrote: > Note the number of descriptors select() is waiting on in independent of t= he > number of files. We should be able to inotify_init() earlier in the proce= ss > to avoid this issue. I'll have a look. Good idea! This could make it work instead of throwing an error. =20 Nevertheless, the boundary check should be added anyway as long as we use= =20 FD_SET(), because the number of first available file descriptor can still be controlled from outside. Kamil From debbugs-submit-bounces@debbugs.gnu.org Thu Jun 24 11:50:26 2021 Received: (at 49209) by debbugs.gnu.org; 24 Jun 2021 15:50:26 +0000 Received: from localhost ([127.0.0.1]:43869 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lwRcY-0007hC-Nj for submit@debbugs.gnu.org; Thu, 24 Jun 2021 11:50:26 -0400 Received: from zimbra.cs.ucla.edu ([131.179.128.68]:49038) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lwRcX-0007gp-2S for 49209@debbugs.gnu.org; Thu, 24 Jun 2021 11:50:26 -0400 Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id E82B5160068; Thu, 24 Jun 2021 08:50:17 -0700 (PDT) Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id M9PL3cxGVS25; Thu, 24 Jun 2021 08:50:17 -0700 (PDT) Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 1526E1600EF; Thu, 24 Jun 2021 08:50:17 -0700 (PDT) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id mBBve1IITjRK; Thu, 24 Jun 2021 08:50:17 -0700 (PDT) Received: from [192.168.1.9] (cpe-172-91-119-151.socal.res.rr.com [172.91.119.151]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id DFC47160068; Thu, 24 Jun 2021 08:50:16 -0700 (PDT) Subject: Re: bug#49209: coreutils: stack out-of-bounds write in tail --follow To: =?UTF-8?Q?P=c3=a1draig_Brady?= , Kamil Dudka , 49209@debbugs.gnu.org References: <1840531.tdWV9SEqCh@nbkamil> From: Paul Eggert Organization: UCLA Computer Science Department Message-ID: Date: Thu, 24 Jun 2021 08:50:16 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 49209 Cc: sbroz@redhat.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) On 6/24/21 7:50 AM, P=C3=A1draig Brady wrote: > We should be able to inotify_init() earlier in the process to avoid thi= s=20 > issue. inotify_init can return 1025 even if called first thing, so we also need=20 to dup2 the result of early inotify_init down to 3 (or whatever), or at=20 least to check that it's less than 1024. Choosing 3 is a tricky=20 business, since it's not clear what fds the C library actually needs. From debbugs-submit-bounces@debbugs.gnu.org Sat Jun 26 21:48:02 2021 Received: (at 49209-done) by debbugs.gnu.org; 27 Jun 2021 01:48:02 +0000 Received: from localhost ([127.0.0.1]:48798 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lxJtx-00033R-3o for submit@debbugs.gnu.org; Sat, 26 Jun 2021 21:48:02 -0400 Received: from zimbra.cs.ucla.edu ([131.179.128.68]:50634) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lxJtt-00033B-AF for 49209-done@debbugs.gnu.org; Sat, 26 Jun 2021 21:47:59 -0400 Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 299261600FB; Sat, 26 Jun 2021 18:47:51 -0700 (PDT) Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id sBN-9vAza102; Sat, 26 Jun 2021 18:47:48 -0700 (PDT) Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 4A3C8160102; Sat, 26 Jun 2021 18:47:48 -0700 (PDT) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id iRk1fk7bG07x; Sat, 26 Jun 2021 18:47:47 -0700 (PDT) Received: from [192.168.1.9] (cpe-172-91-119-151.socal.res.rr.com [172.91.119.151]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id B86711600FB; Sat, 26 Jun 2021 18:47:47 -0700 (PDT) From: Paul Eggert To: Stepan Broz References: <1840531.tdWV9SEqCh@nbkamil> Organization: UCLA Computer Science Department Subject: Re: bug#49209: coreutils: stack out-of-bounds write in tail --follow Message-ID: Date: Sat, 26 Jun 2021 18:47:46 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------7FF6C760B3CD89CE9984B3CB" Content-Language: en-US X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 49209-done Cc: Kamil Dudka , =?UTF-8?Q?P=c3=a1draig_Brady?= , 49209-done@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) This is a multi-part message in MIME format. --------------7FF6C760B3CD89CE9984B3CB Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable On 6/24/21 8:50 AM, Paul Eggert wrote: > inotify_init can return 1025 even if called first thing, so we also nee= d=20 > to dup2 the result of early inotify_init down to 3 (or whatever), or at= =20 > least to check that it's less than 1024. Choosing 3 is a tricky=20 > business, since it's not clear what fds the C library actually needs. When looking into this I decided it was cleaner to fix coreutils by=20 using 'poll' instead of 'select', as Kamil suggested. I installed the=20 attached patches to do that. The last patch fixes the bug. Thanks for reporting the problem. --------------7FF6C760B3CD89CE9984B3CB Content-Type: text/x-patch; charset=UTF-8; name="0001-maint-while-1-while-true.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="0001-maint-while-1-while-true.patch" =46rom 145707949f9479f00dce41f479549f7629d7d0c9 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sat, 26 Jun 2021 18:23:52 -0700 Subject: [PATCH 1/3] =3D?UTF-8?q?maint:=3D20while=3D20(1)=3D20=3DE2=3D86=3D= 92=3D20while=3D20?=3D =3D?UTF-8?q?(true)?=3D MIME-Version: 1.0 Content-Type: text/plain; charset=3DUTF-8 Content-Transfer-Encoding: 8bit --- src/basenc.c | 2 +- src/chcon.c | 2 +- src/chmod.c | 2 +- src/chown-core.c | 2 +- src/csplit.c | 2 +- src/cut.c | 2 +- src/date.c | 2 +- src/dd.c | 2 +- src/dircolors.c | 2 +- src/du.c | 2 +- src/expr.c | 12 ++++++------ src/head.c | 4 ++-- src/join.c | 2 +- src/ls.c | 4 ++-- src/nproc.c | 2 +- src/od.c | 6 +++--- src/ptx.c | 2 +- src/pwd.c | 6 +++--- src/realpath.c | 2 +- src/remove.c | 2 +- src/rmdir.c | 2 +- src/runcon.c | 2 +- src/sort.c | 2 +- src/sum.c | 2 +- src/system.h | 2 +- src/tac-pipe.c | 6 +++--- src/tac.c | 2 +- src/tail.c | 12 ++++++------ src/tsort.c | 2 +- 29 files changed, 47 insertions(+), 47 deletions(-) diff --git a/src/basenc.c b/src/basenc.c index 7e22363bf..5c97a3652 100644 --- a/src/basenc.c +++ b/src/basenc.c @@ -605,7 +605,7 @@ z85_encode (char const *restrict in, size_t inlen, unsigned int val; size_t outidx =3D 0; =20 - while (1) + while (true) { if (inlen =3D=3D 0) { diff --git a/src/chcon.c b/src/chcon.c index dc2258de0..88010848e 100644 --- a/src/chcon.c +++ b/src/chcon.c @@ -315,7 +315,7 @@ process_files (char **files, int bit_flags) =20 FTS *fts =3D xfts_open (files, bit_flags, NULL); =20 - while (1) + while (true) { FTSENT *ent; =20 diff --git a/src/chmod.c b/src/chmod.c index 78d9c9cba..160a0c537 100644 --- a/src/chmod.c +++ b/src/chmod.c @@ -335,7 +335,7 @@ process_files (char **files, int bit_flags) =20 FTS *fts =3D xfts_open (files, bit_flags, NULL); =20 - while (1) + while (true) { FTSENT *ent; =20 diff --git a/src/chown-core.c b/src/chown-core.c index a0b2f670f..4d816de4a 100644 --- a/src/chown-core.c +++ b/src/chown-core.c @@ -524,7 +524,7 @@ chown_files (char **files, int bit_flags, =20 FTS *fts =3D xfts_open (files, bit_flags | stat_flags, NULL); =20 - while (1) + while (true) { FTSENT *ent; =20 diff --git a/src/csplit.c b/src/csplit.c index f188e8894..e1fb66ed2 100644 --- a/src/csplit.c +++ b/src/csplit.c @@ -496,7 +496,7 @@ load_buffer (void) if (bytes_wanted < hold_count) bytes_wanted =3D hold_count; =20 - while (1) + while (true) { b =3D get_new_buffer (bytes_wanted); bytes_avail =3D b->bytes_alloc; /* Size of buffer returned. */ diff --git a/src/cut.c b/src/cut.c index 78b82c80e..f4d44c211 100644 --- a/src/cut.c +++ b/src/cut.c @@ -306,7 +306,7 @@ cut_fields (FILE *stream) That is because a non-delimited line has exactly one field. */ buffer_first_field =3D (suppress_non_delimited ^ !print_kth (1)); =20 - while (1) + while (true) { if (field_idx =3D=3D 1 && buffer_first_field) { diff --git a/src/date.c b/src/date.c index d5eebaf25..4a7a4e243 100644 --- a/src/date.c +++ b/src/date.c @@ -313,7 +313,7 @@ batch_convert (char const *input_filename, char const= *format, line =3D NULL; buflen =3D 0; ok =3D true; - while (1) + while (true) { ssize_t line_length =3D getline (&line, &buflen, in_stream); if (line_length < 0) diff --git a/src/dd.c b/src/dd.c index d284357a4..fc5108f8b 100644 --- a/src/dd.c +++ b/src/dd.c @@ -2179,7 +2179,7 @@ dd_copy (void) alloc_ibuf (); alloc_obuf (); =20 - while (1) + while (true) { if (status_level =3D=3D STATUS_PROGRESS) { diff --git a/src/dircolors.c b/src/dircolors.c index fea0cdf01..b765ded9f 100644 --- a/src/dircolors.c +++ b/src/dircolors.c @@ -253,7 +253,7 @@ dc_parse_stream (FILE *fp, char const *filename) if (term =3D=3D NULL || *term =3D=3D '\0') term =3D "none"; =20 - while (1) + while (true) { char *keywd, *arg; bool unrecognized; diff --git a/src/du.c b/src/du.c index d4760a36c..efd519706 100644 --- a/src/du.c +++ b/src/du.c @@ -684,7 +684,7 @@ du_files (char **files, int bit_flags) { FTS *fts =3D xfts_open (files, bit_flags, NULL); =20 - while (1) + while (true) { FTSENT *ent; =20 diff --git a/src/expr.c b/src/expr.c index ec76f7607..41185a8f8 100644 --- a/src/expr.c +++ b/src/expr.c @@ -781,7 +781,7 @@ eval5 (bool evaluate) trace ("eval5"); #endif l =3D eval6 (evaluate); - while (1) + while (true) { if (nextarg (":")) { @@ -812,7 +812,7 @@ eval4 (bool evaluate) trace ("eval4"); #endif l =3D eval5 (evaluate); - while (1) + while (true) { if (nextarg ("*")) fxn =3D multiply; @@ -851,7 +851,7 @@ eval3 (bool evaluate) trace ("eval3"); #endif l =3D eval4 (evaluate); - while (1) + while (true) { if (nextarg ("+")) fxn =3D plus; @@ -881,7 +881,7 @@ eval2 (bool evaluate) trace ("eval2"); #endif l =3D eval3 (evaluate); - while (1) + while (true) { VALUE *r; enum @@ -960,7 +960,7 @@ eval1 (bool evaluate) trace ("eval1"); #endif l =3D eval2 (evaluate); - while (1) + while (true) { if (nextarg ("&")) { @@ -991,7 +991,7 @@ eval (bool evaluate) trace ("eval"); #endif l =3D eval1 (evaluate); - while (1) + while (true) { if (nextarg ("|")) { diff --git a/src/head.c b/src/head.c index 844c5636a..7b2a44041 100644 --- a/src/head.c +++ b/src/head.c @@ -520,7 +520,7 @@ elide_tail_lines_pipe (char const *filename, int fd, = uintmax_t n_elide, /* Always read into a fresh buffer. Read, (producing no output) until we've accumulated at least n_elide newlines, or until EOF, whichever comes first. */ - while (1) + while (true) { n_read =3D safe_read (fd, tmp->buffer, BUFSIZ); if (n_read =3D=3D 0 || n_read =3D=3D SAFE_READ_ERROR) @@ -676,7 +676,7 @@ elide_tail_lines_seekable (char const *pretty_filenam= e, int fd, if (n_lines && bytes_read && buffer[bytes_read - 1] !=3D line_end) --n_lines; =20 - while (1) + while (true) { /* Scan backward, counting the newlines in this bufferfull. */ =20 diff --git a/src/join.c b/src/join.c index 57f4665bc..f22ffda53 100644 --- a/src/join.c +++ b/src/join.c @@ -598,7 +598,7 @@ prjoin (struct line const *line1, struct line const *= line2) const struct outlist *o; =20 o =3D outlist; - while (1) + while (true) { if (o->file =3D=3D 0) { diff --git a/src/ls.c b/src/ls.c index 4586b5ecc..1b20f17fe 100644 --- a/src/ls.c +++ b/src/ls.c @@ -2980,7 +2980,7 @@ print_dir (char const *name, char const *realname, = bool command_line_arg) /* Read the directory entries, and insert the subfiles into the 'cwd_f= ile' table. */ =20 - while (1) + while (true) { /* Set errno to zero so we can distinguish between a readdir failu= re and when readdir simply finds that there are no more entries. = */ @@ -5112,7 +5112,7 @@ print_many_per_line (void) size_t pos =3D 0; =20 /* Print the next row. */ - while (1) + while (true) { struct fileinfo const *f =3D sorted_file[filesno]; size_t name_length =3D length_of_file_name_and_frills (f); diff --git a/src/nproc.c b/src/nproc.c index e06b7a431..55a51bfbe 100644 --- a/src/nproc.c +++ b/src/nproc.c @@ -86,7 +86,7 @@ main (int argc, char **argv) =20 enum nproc_query mode =3D NPROC_CURRENT_OVERRIDABLE; =20 - while (1) + while (true) { int c =3D getopt_long (argc, argv, "", longopts, NULL); if (c =3D=3D -1) diff --git a/src/od.c b/src/od.c index 5b2d507a1..f04e0ccb7 100644 --- a/src/od.c +++ b/src/od.c @@ -1387,7 +1387,7 @@ dump (void) =20 if (limit_bytes_to_format) { - while (1) + while (true) { size_t n_needed; if (current_offset >=3D end_offset) @@ -1409,7 +1409,7 @@ dump (void) } else { - while (1) + while (true) { ok &=3D read_block (bytes_per_block, block[idx], &n_bytes_read= ); if (n_bytes_read < bytes_per_block) @@ -1462,7 +1462,7 @@ dump_strings (void) uintmax_t address =3D n_bytes_to_skip; bool ok =3D true; =20 - while (1) + while (true) { size_t i; int c; diff --git a/src/ptx.c b/src/ptx.c index fc4d0912f..c488b1192 100644 --- a/src/ptx.c +++ b/src/ptx.c @@ -837,7 +837,7 @@ find_occurs_in_text (int file_index) /* Read and process a single input line or sentence, one word at a= time. */ =20 - while (1) + while (true) { if (word_regex.string) =20 diff --git a/src/pwd.c b/src/pwd.c index 3a18c3d81..5dfda31bd 100644 --- a/src/pwd.c +++ b/src/pwd.c @@ -178,7 +178,7 @@ find_dir_entry (struct stat *dot_sb, struct file_name= *file_name, use_lstat =3D (parent_sb.st_dev !=3D dot_sb->st_dev); =20 found =3D false; - while (1) + while (true) { struct dirent const *dp; struct stat ent_sb; @@ -279,7 +279,7 @@ robust_getcwd (struct file_name *file_name) if (stat (".", &dot_sb) < 0) die (EXIT_FAILURE, errno, _("failed to stat %s"), quoteaf (".")); =20 - while (1) + while (true) { /* If we've reached the root, we're done. */ if (SAME_INODE (dot_sb, *root_dev_ino)) @@ -340,7 +340,7 @@ main (int argc, char **argv) =20 atexit (close_stdout); =20 - while (1) + while (true) { int c =3D getopt_long (argc, argv, "LP", longopts, NULL); if (c =3D=3D -1) diff --git a/src/realpath.c b/src/realpath.c index 4c9ffbcb4..8e4aabfdf 100644 --- a/src/realpath.c +++ b/src/realpath.c @@ -185,7 +185,7 @@ main (int argc, char **argv) =20 atexit (close_stdout); =20 - while (1) + while (true) { int c =3D getopt_long (argc, argv, "eLmPqsz", longopts, NULL); if (c =3D=3D -1) diff --git a/src/remove.c b/src/remove.c index da380e0a7..5dd0479e6 100644 --- a/src/remove.c +++ b/src/remove.c @@ -590,7 +590,7 @@ rm (char *const *file, struct rm_options const *x) =20 FTS *fts =3D xfts_open (file, bit_flags, NULL); =20 - while (1) + while (true) { FTSENT *ent; =20 diff --git a/src/rmdir.c b/src/rmdir.c index 3a9911ff0..149d4659a 100644 --- a/src/rmdir.c +++ b/src/rmdir.c @@ -117,7 +117,7 @@ remove_parents (char *dir) bool ok =3D true; =20 strip_trailing_slashes (dir); - while (1) + while (true) { slash =3D strrchr (dir, '/'); if (slash =3D=3D NULL) diff --git a/src/runcon.c b/src/runcon.c index fda8bb398..716170092 100644 --- a/src/runcon.c +++ b/src/runcon.c @@ -124,7 +124,7 @@ main (int argc, char **argv) =20 atexit (close_stdout); =20 - while (1) + while (true) { int option_index =3D 0; int c =3D getopt_long (argc, argv, "+r:t:u:l:c", long_options, diff --git a/src/sort.c b/src/sort.c index d98796b96..451bfe08a 100644 --- a/src/sort.c +++ b/src/sort.c @@ -3552,7 +3552,7 @@ static void merge_loop (struct merge_node_queue *queue, size_t total_lines, FILE *tfp, char const *temp_output) { - while (1) + while (true) { struct merge_node *node =3D queue_pop (queue); =20 diff --git a/src/sum.c b/src/sum.c index 218db808d..f9641dbb1 100644 --- a/src/sum.c +++ b/src/sum.c @@ -179,7 +179,7 @@ sysv_sum_file (char const *file, int print_name) } } =20 - while (1) + while (true) { size_t bytes_read =3D safe_read (fd, buf, sizeof buf); =20 diff --git a/src/system.h b/src/system.h index 676f5f562..ce264b121 100644 --- a/src/system.h +++ b/src/system.h @@ -277,7 +277,7 @@ dot_or_dotdot (char const *file_name) static inline struct dirent const * readdir_ignoring_dot_and_dotdot (DIR *dirp) { - while (1) + while (true) { struct dirent const *dp =3D readdir (dirp); if (dp =3D=3D NULL || ! dot_or_dotdot (dp->d_name)) diff --git a/src/tac-pipe.c b/src/tac-pipe.c index 5fefd631b..5192607d9 100644 --- a/src/tac-pipe.c +++ b/src/tac-pipe.c @@ -58,7 +58,7 @@ buf_init_from_stdin (Buf *x, char eol_byte) #define OBS (&(x->obs)) obstack_init (OBS); =20 - while (1) + while (true) { char *buf =3D (char *) malloc (BUFFER_SIZE); size_t bytes_read; @@ -185,7 +185,7 @@ find_bol (const Buf *x, tmp =3D line_ptr_decrement (x, last_bol); last_bol_ptr =3D tmp.ptr; i =3D tmp.i; - while (1) + while (true) { char *nl =3D memrchr (x->p[i].start, last_bol_ptr, eol_byte); if (nl) @@ -250,7 +250,7 @@ tac_mem () bol.i =3D x.n_bufs - 1; bol.ptr =3D ONE_PAST_END (&x, bol.i); =20 - while (1) + while (true) { Line_ptr new_bol; if (! find_bol (&x, &bol, &new_bol, eol_byte)) diff --git a/src/tac.c b/src/tac.c index 1a01c005e..cbc3deac9 100644 --- a/src/tac.c +++ b/src/tac.c @@ -505,7 +505,7 @@ copy_to_temp (FILE **g_tmp, char **g_tempfile, int in= put_fd, char const *file) if (!temp_stream (&fp, &file_name)) return -1; =20 - while (1) + while (true) { size_t bytes_read =3D safe_read (input_fd, G_buffer, read_size); if (bytes_read =3D=3D 0) diff --git a/src/tail.c b/src/tail.c index b91fcfd82..7c3ae22d9 100644 --- a/src/tail.c +++ b/src/tail.c @@ -453,7 +453,7 @@ dump_remainder (bool want_header, char const *pretty_= filename, int fd, uintmax_t n_remaining =3D n_bytes; =20 n_written =3D 0; - while (1) + while (true) { char buffer[BUFSIZ]; size_t n =3D MIN (n_remaining, BUFSIZ); @@ -641,7 +641,7 @@ pipe_lines (char const *pretty_filename, int fd, uint= max_t n_lines, tmp =3D xmalloc (sizeof (LBUFFER)); =20 /* Input is always read into a fresh buffer. */ - while (1) + while (true) { n_read =3D safe_read (fd, tmp->buffer, BUFSIZ); if (n_read =3D=3D 0 || n_read =3D=3D SAFE_READ_ERROR) @@ -780,7 +780,7 @@ pipe_bytes (char const *pretty_filename, int fd, uint= max_t n_bytes, tmp =3D xmalloc (sizeof (CBUFFER)); =20 /* Input is always read into a fresh buffer. */ - while (1) + while (true) { n_read =3D safe_read (fd, tmp->buffer, BUFSIZ); if (n_read =3D=3D 0 || n_read =3D=3D SAFE_READ_ERROR) @@ -900,7 +900,7 @@ start_lines (char const *pretty_filename, int fd, uin= tmax_t n_lines, if (n_lines =3D=3D 0) return 0; =20 - while (1) + while (true) { char buffer[BUFSIZ]; size_t bytes_read =3D safe_read (fd, buffer, BUFSIZ); @@ -1160,7 +1160,7 @@ tail_forever (struct File_spec *f, size_t n_files, = double sleep_interval) =20 last =3D n_files - 1; =20 - while (1) + while (true) { size_t i; bool any_input =3D false; @@ -1617,7 +1617,7 @@ tail_forever_inotify (int wd, struct File_spec *f, = size_t n_files, ensure that watched files can be re-added when following by name. This loop blocks on the 'safe_read' call until a new event is notif= ied. But when --pid=3DP is specified, tail usually waits via the select.= */ - while (1) + while (true) { struct File_spec *fspec; struct inotify_event *ev; diff --git a/src/tsort.c b/src/tsort.c index 92bd1b2af..33b3419c0 100644 --- a/src/tsort.c +++ b/src/tsort.c @@ -452,7 +452,7 @@ tsort (char const *file) =20 init_tokenbuffer (&tokenbuffer); =20 - while (1) + while (true) { /* T2. Next Relation. */ size_t len =3D readtoken (stdin, DELIM, sizeof (DELIM) - 1, &token= buffer); --=20 2.30.2 --------------7FF6C760B3CD89CE9984B3CB Content-Type: text/x-patch; charset=UTF-8; name="0002-tail-fix-abuse2-test-race.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="0002-tail-fix-abuse2-test-race.patch" =46rom bce0fe7fa095edfe52d0724d9287a9043346316b Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sat, 26 Jun 2021 18:23:52 -0700 Subject: [PATCH 2/3] tail: fix abuse2 test race MIME-Version: 1.0 Content-Type: text/plain; charset=3DUTF-8 Content-Transfer-Encoding: 8bit * tests/tail-2/inotify-hash-abuse2.sh (fastpoll): Fix race where tailed file =E2=80=98f=E2=80=99 temporarily did not exist.= --- tests/tail-2/inotify-hash-abuse2.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/tail-2/inotify-hash-abuse2.sh b/tests/tail-2/inotify-h= ash-abuse2.sh index 2c40236af..4b36b7a33 100755 --- a/tests/tail-2/inotify-hash-abuse2.sh +++ b/tests/tail-2/inotify-hash-abuse2.sh @@ -33,8 +33,8 @@ for mode in '' '---disable-inotify'; do =20 for i in $(seq 200); do kill -0 $pid || break; - mv f g - touch f + touch g + mv g f done =20 # Ensure tail hasn't aborted --=20 2.30.2 --------------7FF6C760B3CD89CE9984B3CB Content-Type: text/x-patch; charset=UTF-8; name="0003-tail-use-poll-not-select.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="0003-tail-use-poll-not-select.patch" =46rom 3f83f82d2612d0c6ea8f1983350e85ef3cc0fc7a Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sat, 26 Jun 2021 18:23:52 -0700 Subject: [PATCH 3/3] tail: use poll, not select MIME-Version: 1.0 Content-Type: text/plain; charset=3DUTF-8 Content-Transfer-Encoding: 8bit This fixes an unlikely stack out-of-bounds write reported by Stepan Broz via Kamil Dudka (Bug#49209). * bootstrap.conf (gnulib_modules): Replace select with poll. * src/tail.c: Do not include . [!_AIX]: Include poll.h. (check_output_alive) [!_AIX]: Use poll instead of select. (tail_forever_inotify): Likewise. Simplify logic, as there is no need for a =E2=80=98while (len <=3D evbuf_off)=E2=80=99 loop. --- NEWS | 4 ++ bootstrap.conf | 2 +- src/tail.c | 100 +++++++++++++++++++------------------------------ 3 files changed, 44 insertions(+), 62 deletions(-) diff --git a/NEWS b/NEWS index 02b57470f..97532d3d0 100644 --- a/NEWS +++ b/NEWS @@ -43,6 +43,10 @@ GNU coreutils NEWS = -*- outline -*- Previously a chunk starting after 128KiB, output the wrong part of the= file. [bug introduced in coreutils-8.26] =20 + tail -f no longer overruns a stack buffer when given too many files + to follow and ulimit -n exceeds 1024. + [bug introduced in coreutils-7.5] + tr no longer crashes when using --complement with certain invalid combinations of case character classes. [bug introduced in coreutils-8.6] diff --git a/bootstrap.conf b/bootstrap.conf index daea2824c..e7ed7e5ff 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -193,6 +193,7 @@ gnulib_modules=3D" physmem pipe-posix pipe2 + poll posix-shell posixtm posixver @@ -227,7 +228,6 @@ gnulib_modules=3D" save-cwd savedir savewd - select selinux-at setenv settime diff --git a/src/tail.c b/src/tail.c index 7c3ae22d9..99977afa7 100644 --- a/src/tail.c +++ b/src/tail.c @@ -28,12 +28,9 @@ #include #include #include -#include +#include #include #include -#ifdef _AIX -# include -#endif =20 #include "system.h" #include "argmatch.h" @@ -351,27 +348,12 @@ check_output_alive (void) if (! monitor_output) return; =20 -#ifdef _AIX - /* select on AIX was seen to give a readable event immediately. */ struct pollfd pfd; pfd.fd =3D STDOUT_FILENO; pfd.events =3D POLLERR; =20 if (poll (&pfd, 1, 0) >=3D 0 && (pfd.revents & POLLERR)) die_pipe (); -#else - struct timeval delay; - delay.tv_sec =3D delay.tv_usec =3D 0; - - fd_set rfd; - FD_ZERO (&rfd); - FD_SET (STDOUT_FILENO, &rfd); - - /* readable event on STDOUT is equivalent to POLLERR, - and implies an error condition on output like broken pipe. */ - if (select (STDOUT_FILENO + 1, &rfd, NULL, NULL, &delay) =3D=3D 1) - die_pipe (); -#endif } =20 static bool @@ -1616,7 +1598,7 @@ tail_forever_inotify (int wd, struct File_spec *f, = size_t n_files, /* Wait for inotify events and handle them. Events on directories ensure that watched files can be re-added when following by name. This loop blocks on the 'safe_read' call until a new event is notif= ied. - But when --pid=3DP is specified, tail usually waits via the select.= */ + But when --pid=3DP is specified, tail usually waits via poll. */ while (true) { struct File_spec *fspec; @@ -1636,54 +1618,51 @@ tail_forever_inotify (int wd, struct File_spec *f= , size_t n_files, return false; } =20 - /* When watching a PID, ensure that a read from WD will not block - indefinitely. */ - while (len <=3D evbuf_off) + if (len <=3D evbuf_off) { - struct timeval delay; /* how long to wait for file changes. *= / + /* Poll for inotify events. When watching a PID, ensure + that a read from WD will not block indefinitely. + If MONITOR_OUTPUT, also poll for a broken output pipe. */ =20 - if (pid) + int file_change; + struct pollfd pfd[2]; + do { - if (writer_is_dead) - exit (EXIT_SUCCESS); + /* How many ms to wait for changes. -1 means wait forever= =2E */ + int delay =3D -1; =20 - writer_is_dead =3D (kill (pid, 0) !=3D 0 && errno !=3D EPE= RM); - - if (writer_is_dead) - delay.tv_sec =3D delay.tv_usec =3D 0; - else + if (pid) { - delay.tv_sec =3D (time_t) sleep_interval; - delay.tv_usec =3D 1000000 * (sleep_interval - delay.tv= _sec); + if (writer_is_dead) + exit (EXIT_SUCCESS); + + writer_is_dead =3D (kill (pid, 0) !=3D 0 && errno !=3D= EPERM); + + if (writer_is_dead || sleep_interval <=3D 0) + delay =3D 0; + else if (sleep_interval < INT_MAX / 1000 - 1) + { + /* delay =3D ceil (sleep_interval * 1000), sans li= bm. */ + double ddelay =3D sleep_interval * 1000; + delay =3D ddelay; + delay +=3D delay < ddelay; + } } + + pfd[0].fd =3D wd; + pfd[0].events =3D POLLIN; + pfd[1].fd =3D STDOUT_FILENO; + pfd[1].events =3D pfd[1].revents =3D 0; + file_change =3D poll (pfd, monitor_output + 1, delay); } + while (file_change =3D=3D 0); =20 - fd_set rfd; - FD_ZERO (&rfd); - FD_SET (wd, &rfd); - if (monitor_output) - FD_SET (STDOUT_FILENO, &rfd); - - int file_change =3D select (MAX (wd, STDOUT_FILENO) + 1, - &rfd, NULL, NULL, pid ? &delay: NUL= L); - - if (file_change =3D=3D 0) - continue; - else if (file_change =3D=3D -1) - die (EXIT_FAILURE, errno, - _("error waiting for inotify and output events")); - else if (FD_ISSET (STDOUT_FILENO, &rfd)) - { - /* readable event on STDOUT is equivalent to POLLERR, - and implies an error on output like broken pipe. */ - die_pipe (); - } - else - break; - } + if (file_change < 0) + die (EXIT_FAILURE, errno, + _("error waiting for inotify and output events")); + if (pfd[1].revents) + die_pipe (); =20 - if (len <=3D evbuf_off) - { len =3D safe_read (wd, evbuf, evlen); evbuf_off =3D 0; =20 @@ -2444,8 +2423,7 @@ main (int argc, char **argv) if (forever && ignore_fifo_and_pipe (F, n_files)) { /* If stdout is a fifo or pipe, then monitor it - so that we exit if the reader goes away. - Note select() on a regular file is always readable. */ + so that we exit if the reader goes away. */ struct stat out_stat; if (fstat (STDOUT_FILENO, &out_stat) < 0) die (EXIT_FAILURE, errno, _("standard output")); --=20 2.30.2 --------------7FF6C760B3CD89CE9984B3CB-- From debbugs-submit-bounces@debbugs.gnu.org Mon Jun 28 11:10:08 2021 Received: (at 49209-done) by debbugs.gnu.org; 28 Jun 2021 15:10:08 +0000 Received: from localhost ([127.0.0.1]:52249 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lxstk-0001Pu-5d for submit@debbugs.gnu.org; Mon, 28 Jun 2021 11:10:08 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:41519) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lxsth-0001Pi-6p for 49209-done@debbugs.gnu.org; Mon, 28 Jun 2021 11:10:06 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1624893005; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=a4GFDmIUEoXOHRxAZKWyNWEsKxQiTikZjc/oUdgqByY=; b=T30jYNLVWPDDuM1L+A3smughF37axg/ZyCkXr1eBc7tTMNiiRSwA2QQ/CF/Y1Omgf9eBil bkfbaiIsimxE616Ly3sSN8zMAtWg1OP9Yp8MfvOU7Uodyken8KK7lm4gsEbsLlIcXaijzp OLamcQnNsZ9Xs3hRhKLsSEkQ9F7M4GI= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-41-_O-qTbe2P5mBXcSjmOU_DQ-1; Mon, 28 Jun 2021 11:10:03 -0400 X-MC-Unique: _O-qTbe2P5mBXcSjmOU_DQ-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 1DDAB8015F8; Mon, 28 Jun 2021 15:10:02 +0000 (UTC) Received: from nbkamil.localnet (unknown [10.43.7.71]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1290010013D6; Mon, 28 Jun 2021 15:10:00 +0000 (UTC) From: Kamil Dudka To: Paul Eggert Subject: Re: bug#49209: coreutils: stack out-of-bounds write in tail --follow Date: Mon, 28 Jun 2021 17:10:00 +0200 Message-ID: <4324241.LvFx2qVVIh@nbkamil> In-Reply-To: References: <1840531.tdWV9SEqCh@nbkamil> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=kdudka@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 49209-done Cc: Stepan Broz , =?ISO-8859-1?Q?P=E1draig?= Brady , 49209-done@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) On Sunday, June 27, 2021 3:47:46 AM CEST Paul Eggert wrote: > When looking into this I decided it was cleaner to fix coreutils by > using 'poll' instead of 'select', as Kamil suggested. I installed the > attached patches to do that. The last patch fixes the bug. This works for me. Thank you for taking care of the fix! Kamil From debbugs-submit-bounces@debbugs.gnu.org Mon Jun 28 19:10:54 2021 Received: (at 49209) by debbugs.gnu.org; 28 Jun 2021 23:10:54 +0000 Received: from localhost ([127.0.0.1]:53250 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ly0Oz-0003Sm-Sn for submit@debbugs.gnu.org; Mon, 28 Jun 2021 19:10:54 -0400 Received: from mail-wr1-f49.google.com ([209.85.221.49]:39617) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ly0Ow-0003ST-J4 for 49209@debbugs.gnu.org; Mon, 28 Jun 2021 19:10:53 -0400 Received: by mail-wr1-f49.google.com with SMTP id b3so23274870wrm.6 for <49209@debbugs.gnu.org>; Mon, 28 Jun 2021 16:10:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:subject:to:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=4bQ2BmuG+2lwAXgBY7Mnd5C93LOC1UMZ7wbsGuf6eMY=; b=EpyokGNh1iGvqmRqUoxI1q9ESypKEd7kVNxbTLbsnmrYoT/b9Wn6JHtGtRt/AArpRt ls3BLrFXJ068DxkmXcHP3zTNxpoeHEJTrIr1sIgFXv9td/uCZZ2qy8TYksxJ0QZDfXSL /8qV32HbjlFc6uIU4p5vAiwekwu1soW0ON7mUnHuxZXzaH7vuSAfRxqot10+W37FqH+S FJxVid4X9Ifo4xrkp9zi2AyV5fCLPhoy6Xh81nFhqXmC83U2mfjcfYHopppNiF7lhrEG 9FK8bFarU9spqy0AbPhpYrPv44JpM68yPUNeub+VoSw33y+/t5jzXDHHPpc6j19dm0nX 3hGw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:subject:to:references:from:message-id :date:user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=4bQ2BmuG+2lwAXgBY7Mnd5C93LOC1UMZ7wbsGuf6eMY=; b=PWoi8OgCIiyDjZmUrgaNC46vZxkjiueFLh8p1K139kLe+4c8Oq988P+a/kbaHCbcHP MYFjuNUKsiH/r/WKv+DWXaBHyjTzlaaQxKcIyQwDH8ffFS3Ui6kDToPJYc0PkQTpeP7B OXBNJw0U1XPX6NbqkoIzxRXZ1gzAviV3LerIbg1+omGxF6BYguq6GzT1mSQ0cpc+wB0/ lsUT5czykUv2ddezdAqsQOzBIEA9vIO4RMo7NSTvw79hLSRcf5ZvYcUGJ5elaN+Xz6Uw j/z6NQfWAZ5xcFZmAnb2C/o0l8w7FPtge0ZWWFRaimGOmoHED4rRFpOmd82XXrzOzn99 d5Sg== X-Gm-Message-State: AOAM530eIil+ymo21apNZG9wLGQZRPxcMWSENPn7LXiuOAai0TveQ5n5 t/VI9smT2jOHYQeSazM+LbM= X-Google-Smtp-Source: ABdhPJzb1xyy5iIKGDqvkLkZpodbkOVB/eks0gPTMUrPNnVCM/yBiMLduHAC6bS3CJfc3u/R6p6VBw== X-Received: by 2002:a5d:6d06:: with SMTP id e6mr31250476wrq.28.1624921844631; Mon, 28 Jun 2021 16:10:44 -0700 (PDT) Received: from localhost.localdomain (86-42-15-3-dynamic.agg2.lod.rsl-rtd.eircom.net. [86.42.15.3]) by smtp.googlemail.com with UTF8SMTPSA id l5sm812445wmq.9.2021.06.28.16.10.43 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 28 Jun 2021 16:10:43 -0700 (PDT) Subject: Re: bug#49209: coreutils: stack out-of-bounds write in tail --follow To: 49209@debbugs.gnu.org, eggert@cs.ucla.edu, kdudka@redhat.com References: <1840531.tdWV9SEqCh@nbkamil> From: =?UTF-8?Q?P=c3=a1draig_Brady?= Message-ID: <16634d1d-567d-61e8-92d4-ba7bc5576a32@draigBrady.com> Date: Tue, 29 Jun 2021 00:10:42 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:84.0) Gecko/20100101 Thunderbird/84.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Score: 0.5 (/) X-Debbugs-Envelope-To: 49209 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.5 (/) On 27/06/2021 02:47, Paul Eggert wrote: > On 6/24/21 8:50 AM, Paul Eggert wrote: > >> inotify_init can return 1025 even if called first thing, so we also need >> to dup2 the result of early inotify_init down to 3 (or whatever), or at >> least to check that it's less than 1024. Choosing 3 is a tricky >> business, since it's not clear what fds the C library actually needs. > > When looking into this I decided it was cleaner to fix coreutils by > using 'poll' instead of 'select', as Kamil suggested. I installed the > attached patches to do that. The last patch fixes the bug. Yes using poll() with the inotify descriptor is cleaner. That's limited to Linux also, so should work fine. For my reference, with the change from select() to poll() in check_output_alive(), we'll need to be more carefully test tests/tail-2/pipe-f.sh on various platforms, especially those where we implement missing poll (mingw, MSVC 14, HP NonStop). If poll() didn't work here for these platforms (and we moved back to using select), we might considering removing poll as a dependency as it would be redundant. thanks for the fix! Pádraig From unknown Thu Aug 14 21:51:07 2025 Received: (at fakecontrol) by fakecontrolmessage; To: internal_control@debbugs.gnu.org From: Debbugs Internal Request Subject: Internal Control Message-Id: bug archived. Date: Tue, 27 Jul 2021 11:24:04 +0000 User-Agent: Fakemail v42.6.9 # This is a fake control message. # # The action: # bug archived. thanks # This fakemail brought to you by your local debbugs # administrator