From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1pIBCt-0000R4-BI for mharc-grub-devel@gnu.org; Wed, 18 Jan 2023 11:22:35 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pIBCr-0000Qt-R2 for grub-devel@gnu.org; Wed, 18 Jan 2023 11:22:33 -0500 Received: from mout.gmx.net ([212.227.17.21]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pIBCq-0005Jv-1d for grub-devel@gnu.org; Wed, 18 Jan 2023 11:22:33 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=s31663417; t=1674058925; bh=i6XL7vcBcBJw54mmSOypcsXhgD3X8ZavnfbC0dyJm+w=; h=X-UI-Sender-Class:Date:From:To:Subject:Cc:References:In-Reply-To; b=a3jhv3pKeLGPmcdc47c4FP4QNkACXKeqT5DRy8ff3P445RVaj+bmO4BHn09kZqNgF QgCRcrpVrYoIjVL/V69N05gsGDpWuKSu0kxbmC06SAtMte+ERM1HaEVysOxqy0J+TI 39ga+qgB9C7UHvDnIZJX7ud5uJ7FwaOV+p/6fvkhz3emsJZN154UGxCBLRoTigBRuP YA/yaiiyMlpDtrEd8vCf753M6RVMLcMRZSRh1W2k5/7tKHhE5FYUwWGLjiLlY9PV9L qw/X7yQgJ5Q25hU4UGKdzfrNJLZHKbK9bjkfdtB8G9aFjNacA0A/DcKB6RdIGkNm26 lI9Lek8eIdaYw== X-UI-Sender-Class: 724b4f7f-cbec-4199-ad4e-598c01a50d3a Received: from scdbackup.webframe.org ([84.179.236.73]) by mail.gmx.net (mrgmx104 [212.227.17.168]) with ESMTPSA (Nemesis) id 1MVNB1-1pBPJK2nB9-00SOdr; Wed, 18 Jan 2023 17:22:05 +0100 Date: Wed, 18 Jan 2023 17:21:14 +0100 From: "Thomas Schmitt" To: grub-devel@gnu.org Subject: Re: [PATCH v2 5/5] fs/iso9660: Prevent skipping CE or ST at start of continuation area Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Cc: lidong.chen@oracle.com, fengtao40@huawei.com, yanan@huawei.com, daniel.kiper@oracle.com, lichenca2005@gmail.com References: <03d12347bcd3308d03a65e1792d12a3a874727ce.1673991546.git.lidong.chen@oracle.com> In-Reply-To: <03d12347bcd3308d03a65e1792d12a3a874727ce.1673991546.git.lidong.chen@oracle.com> Message-Id: <12579393033542124165@scdbackup.webframe.org> X-Provags-ID: V03:K1:d7jkQ6ewlCmCr+pWUTvwrznj+R9kOoVCcqpvCjyXMNNP9inbZMI e8u3bggF2iIwbgBywMNQ4Fbl3EJs2cmRoWk6MVRA9YdZ3r5LYjrG3l2luxc8vsRx0MrcuP3 XFklf2RcuVeuIAFhDU/yTWLyYAojnNLKrrt+TriXl6ux4QdFvjRDtzpVnjNpuo27K6u8PvF AvlbzeSsdUhvwMA9HOS7w== UI-OutboundReport: notjunk:1;M01:P0:E8/bj4IqimM=;c6tT4D8HKMGUDPYiIIwpov0H0i9 /g0X6UoaLQyrClNmTDmwzBTbn7IZOt6XNAvqatzee8AgU7JczmCoqXl6vLc/FUiBSACOhA5qx z6FhKbRbQ2om5A4f6YX39mhW5UC6xFvO+bzALwC6LSfdoxZzLPdgGUSwC5t+xO/HB0+ONzGh3 7KGCv/fB116MxcEH5wEuqmGXkHxeOEtv1RVGnIKhd5vzNQnfk+QbEzXOJcjBE57jmz7Rq6l1d 0AYwyAIKCrsv7gcPIcYkPjluvdXeMhz8PyRXxwKvO/7mCyDD+PrIifzV1xpJGjw2EYibxvitV eZS5uPnsyu0TNb39IjC47vGI2+UHArfpx6P40iKFtxAV6Kpt+se9yYMdkZK85jZ6pMrzT8Wu+ OCvZE8EPP9uzD1S/5L55X6QXOViWAzE5gO9N0SNR4UIv4XxSXoSEWnB7OIs+05AFSinwoJUMW 3mKBQ9q8u3I3bBlgyYqS4cYIlLL6R/eq7miKu8B2cdD5A71d0zkhxnBAzi5INMIswvzeC3hv9 5j0lza1BkCIA3g6YIF+2ZrJj3KOFiGiLfdvMU5DcHDSH/jPVncHs/ALf4darVD/MzRT/ALr4/ nRTJnoWS2edUeL2aNsK7NBJ6ULu88bx/nckqpa4PVJPwQ025UxvE7N+6WbPuwaM/GhMveR+AX 6PRNFv5No91DaXYawfQ62Kreo8gbyJIIDmXHEqgNybsq9gPEvSS+jZsCe9ZRvHd9dfvZZbvb1 tUbS8T4ggPnd5v/6dhg/rwZzq/HLt4I1zcJIx7yZNMpkhq7Bc8Oxp/w2OVsoy5/dLOCICwSkG MGBLFGrxRCen6e0084+naZpjq/gWwCzv3Wh1SLv8D0SMVt2Tk7CpmNzEP2m3zMgTHbIeU4YNl TS0QbjsATYaeMP3G0ansqW880AbtVgu/lpIYJarFsy8CpiuenIgoxkitNuv4k0r1+HC4oPXt+ f33rHBDknm2IfMGy3UcI5aFC2wk= Received-SPF: pass client-ip=212.227.17.21; envelope-from=scdbackup@gmx.net; helo=mout.gmx.net X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 18 Jan 2023 16:22:34 -0000 Hi, On Wed, 18 Jan 2023 08:23:58 +0000 Lidong Chen wr= ote: > If processing of a SUSP CE entry leads to a continuation area which > begins by entry CE or ST, then these entries were skipped without > interpretation. In case of CE this would lead to premature end of > processing the SUSP entries of the file. In case of ST this could > cause following non-SUSP bytes to be interpreted as SUSP entries. > > Signed-off-by: Thomas Schmitt scdbackup@gmx.net > --- > grub-core/fs/iso9660.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/grub-core/fs/iso9660.c b/grub-core/fs/iso9660.c > index ca45b3424..c3ed11f04 100644 > --- a/grub-core/fs/iso9660.c > +++ b/grub-core/fs/iso9660.c > @@ -331,6 +331,18 @@ grub_iso9660_susp_iterate (grub_fshelp_node_t node,= grub_off_t off, > return err; > > entry =3D (struct grub_iso9660_susp_entry *) sua; > + /* > + * The hook function will not process CE or ST. > + * Advancing to the next entry would skip them. > + */ > + ce =3D (struct grub_iso9660_susp_ce *) entry; > + if (ce_block !=3D grub_le_to_cpu32 (ce->blk) << GRUB_ISO9660_LOG2_BL= KSZ > + || off !=3D grub_le_to_cpu32 (ce->off)) > + continue; > + /* > + * Ending up here indicates an endless loop by self reference. > + * So skip this bad CE. > + */ > } > > if (hook (entry, hook_arg)) > -- > 2.35.1 This looks like the part of my retracted v2 proposal which was intended to break the possible endless loop by self-referring CE entries: Date: Fri, 16 Dec 2022 13:57:04 +0100 Message-Id: <31992389627932343306@scdbackup.webframe.org> Subject: Proposal v2: fs/iso9660: Prevent skipping CE or ST at start of continuation area But the problem of CE and ST at the start of a continuation area is not fixed by the above. What remains after postponing the loop breaker is (hopefully) addressed by my proposal v1: Date: Fri, 16 Dec 2022 10:42:13 +0100 Message-Id: <19021389617225107434@scdbackup.webframe.org> Subject: Proposal: fs/iso9660: Prevent skipping CE or ST at start of continuation area =2D-- grub-core/fs/iso9660.c.lidong_chen_patch_2 2022-12-15 11:46:50.6542= 95924 +0 100 +++ grub-core/fs/iso9660.c.lidong_chen_patch_2_ce_fix 2022-12-16 10:05:5= 2.9905 99283 +0100 @@ -336,6 +336,12 @@ grub_iso9660_susp_iterate (grub_fshelp_n } entry =3D (struct grub_iso9660_susp_entry *) sua; + + if (grub_strncmp ((char *) entry->sig, "CE", 2) =3D=3D 0 + || grub_strncmp ((char *) entry->sig, "ST", 2) =3D=3D 0) + /* The hook function will not process CE or ST. + Advancing to the next entry would skip them. */ + continue; } if (hook (entry, hook_arg)) =2D-----------------------------------------------------------------------= --- Bystanders please note that the correctness of my "continue" depends on Lidong Chen's patch 2, which changes the "for"-loop to a "while"-loop with incrementation at the loop's end: - for (entry =3D (struct grub_iso9660_susp_entry *) sua; (char *) entry <= (char *) sua + sua_size - 1 && entry->len > 0; - entry =3D (struct grub_iso9660_susp_entry *) - ((char *) entry + entry->len)) + entry =3D (struct grub_iso9660_susp_entry *) sua; + + while (entry->len > 0) { ... loop content ... + entry =3D (struct grub_iso9660_susp_entry *) ((char *) entry + entr= y->len); + + if (((sua + sua_size) - (char *) entry) < GRUB_ISO9660_SUSP_HEADER_= SZ) + break; } In current grub-core/fs/iso9660.c we would have to goto the start of the loop content, circumventing the incrementation step of "for". Have a nice day :) Thomas