From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1p7Yob-0000Fe-B3 for mharc-grub-devel@gnu.org; Tue, 20 Dec 2022 04:21:40 -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 1p7YoP-0000DI-EN for grub-devel@gnu.org; Tue, 20 Dec 2022 04:21:26 -0500 Received: from mout.gmx.net ([212.227.17.20]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1p7YoN-00008A-8d for grub-devel@gnu.org; Tue, 20 Dec 2022 04:21:25 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=s31663417; t=1671528063; bh=Dp3+Y/PNaXSlkvLwXUsMtq39xN5IEeyJ0AVBAJEb5ow=; h=X-UI-Sender-Class:Date:From:To:Subject:Cc:References:In-Reply-To; b=LDyqjjk3xNIMQUHH3JBSh30TGuGdFihyLvS5Mbr2h4gCvQpXHkH1NdbbZEncrDk7m 538iDl8sysiFxxIa7TMYLTxAE33V4lC6YuG13E4igCyJYaZeWrO3GdgP20eDM/04Fr RBhXOCuQ7tOQsDiGPNcRzx82BkwbghWXUdi5UJFNYRE01v+ac7J/v73WH8dAbuJjCx knYyiZNYG66GiLz1/wS8loHIJ9Up3pT3CR34U7HoYnLjgCXlAWFFgb7IJAMdEAy1bW x8LwyznXrrR9EwEOCTkYyLbCuRUTmHxJ1d3ilISjt8FvzHs+7ZBcaHRnolWudoLBpC 0sVT/X0cbHpog== 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 1MJmKX-1pR0X13tIf-00K7Bt; Tue, 20 Dec 2022 10:21:03 +0100 Date: Tue, 20 Dec 2022 10:21:58 +0100 From: "Thomas Schmitt" To: grub-devel@gnu.org Subject: Re: [PATCH 4/4] fs/iso9660: Incorrect check for entry boudary 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: In-Reply-To: Message-Id: <10428386160010370845@scdbackup.webframe.org> X-Provags-ID: V03:K1:7K73RTlnNbNErhXL4tFQiSICoVgsGSD772CynLY0/z7Tj6T3F49 KRvAilRstutsIrZefoBA6AHvAPEWmgzxeziLMrrB3vrUf7Fyd8BsOoswn+biD34UNUB446n JaGx4LE+7eNwBsROEaXA0eTU79b0Fel3pJRvgDoJbBL9KOUirZT7PmGmF/676TX/S+zSxhw BzwOcxl0e1FiXseA/DmuA== UI-OutboundReport: notjunk:1;M01:P0:bVkbo9WN7Vs=;2mr6dByIoDX0aFRHEKDSUW9txpi laZSMJ6YXnYtEQ3lDrKrVI8bxst2mIcIs/yTKsijSu5JV41yWihDab2qBYqS+KCKtPzkB3pmi LvT1lwkqfl8KE81NOp78SDODO5vWJ1DG+yAj2//GtZHCc25euNGJiBx7aYqZdqoEvU4CLDJlu TyFT/90sx4ltvsJxqvd6a9Wn+GykRs41e8NXWiJtjJuMcqdsj33dMI36HP80qEOAcgOpEVLH+ gcOLhZTy9j1T7sRFNz/rjXJF4IpoqE51YOeRDWDX8P/imMjq9ob1CwdKJ0YIVAL5jKiCtsuAn TTaFt1lwLmTbQOgqWEBFcfTTdeqOBqugGSOAtRXeWDoAx8HKLHbeztXeT6gLyTqE69TjSuRnq KqeYmwBY+Tn6RaYNALYeI+8Ag+ZU51R5g8OUADiDa2n2B5d63osLKBMTs0mYktgYrz8TGB0iL 19Z47PaNJAWIHUv1mxSsC0ENk50mbCKT463Enrxs3P2e7cl5jh+LHIEiVJOPZpTgdXTWwFIww scsEJYoYL6HPJLSuJmf9HhZxHSSvPtrnWG2U0wIUL1SFiDv+tMLBvhx8uEodQ9mkZlDPqgPuG NOOv0IvRgqq3msO4tF5HMfPpJ1KMU6MCwO7s3XbAIwZ3nj7z1ezk9AoDPixHAS+gco8bEpmmk Y+shdSoACHWbsmo/ZomcHBLGxNJNMphEaNVAG39oN4wUp4n7yE3F+qAQFjJ3G3YTIT7TnVzXq LwDxHbGCkVJPSjGUuYgKtxnAfGjjZZJi58DK5dO7qkJX0KpW6HNVlFWap8WfDGxWCUtLcNa1u qtMhuWbRjSSXAJaKmCYE+FcT3V98YrUpr4QagWQryO1z6QA07urjjvqzpd2d7yp4d/aRm+InO UVPMynZch5KM1L2PfR9YF315QoemMORWospdxGL2wtvzrHG7LvgJJAThNAtJIV7JgdPgmDggd 5dCDGA== Received-SPF: pass client-ip=212.227.17.20; 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: Tue, 20 Dec 2022 09:21:28 -0000 Hi, in summary: Your objections to my objections are correct. On Wed, 14 Dec 2022 18:55:05 +0000 Lidong Chen wr= ote: > >> + /* The symlink is not stored as a POSIX symlink, translate it.= */ > >> + while ((pos + GRUB_ISO9660_SUSP_HEADER_SZ) < entry->len) On Dec 15, 2022, at 10:20 AM, Thomas Schmitt wrote: > > This loop is iterating over component records, not SUSP entries. > > Minimum size of a component record is 2. > > > > So i think the appropriate condition is: > > > > while (pos + 1 < entry->len) On Mon, 19 Dec 2022 21:00:23 +0000 Lidong Chen wr= ote: > My understanding is the entry->len is the size of the System Use Field. > the while-loop condition is to make sure the component records are > within the System Use Field boundary. > =E2=80=9CPos=E2=80=9D was initially set to =E2=80=981=E2=80=99 and > incremented by the size of each component record. So, =E2=80=9CPos=E2= =80=9D is relative > to the Component Area, not the System Use Field of =E2=80=9CSL=E2=80=9D. > I think the fix should include the 5 bytes: > > While (pos + 5 < entry->len) You are right. My "+ 1" would be correct if entry->len was the size of the SL entry payload (FLAGS byte and COMPONENT AREA). But further "+ 4" are needed because of the SUSP header size of the SL entry. (One could use "+ GRUB_ISO9660_SUSP_HEADER_SZ" instead of "+ 4" in this case. But at other places you followed my proposal to refer with plain numbers to the description in the RRIP specs. So in the sum "+ 5" is good.) > > I am undecided whether add_part() should be skipped if > > (pos + 2 =3D=3D entry->len) > Do you mean this within the while-loop?: > > If (pos + 2 =3D=3D entry->len) > continue I meant your change of program behavior: - add_part (ctx, (char *) &entry->data[pos + 2], - entry->data[pos + 1]); + if (entry->data[pos + 1] > 0) + { + add_part (ctx, (char *) &entry->data[pos + 2], + entry->data[pos + 1]); + } I obviously made some inappropriate thought jumps when writing (pos + 2 =3D=3D entry->len) Apologies for being confusing. (It's not only about the end of the SL entry but about the size of any component record, regardless of its position in the entry's payload. Aggravating is that the condition i mentioned is wrong by the missing "+ 4" for the SUSP header, even if it was about the SL entry's end.) Whatever, you already decided not to change program behavior by skipping over empty text components. > > Open issues: > > [...] > > The while-condition of the component loop should neither use sizeof (*= entry) > > nor GRUB_ISO9660_SUSP_HEADER_SZ, but plain 1, because that's the minim= um > > size of an SL component record minus 1. > > Probably this should be a separate bug-fix patch to be applied before = this > > patch [4/4]. So i now retract this issue. > > The change of program behavior by ignoring empty link target path comp= onents > > should be mentioned in the commit message. This issue is now obsolete. Have a nice day :) Thomas