From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1mfP0E-0001bJ-0G for mharc-grub-devel@gnu.org; Tue, 26 Oct 2021 12:08:42 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:57984) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mfP0B-0001Xa-8n for grub-devel@gnu.org; Tue, 26 Oct 2021 12:08:39 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:57652) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mfP08-0004Cg-Tn for grub-devel@gnu.org; Tue, 26 Oct 2021 12:08:38 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1635264516; 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: in-reply-to:in-reply-to:references:references; bh=ESqWTMl1zlyFBwJDVTKxmCOcELypdKN0pxqJlxhK710=; b=gaf25ILIdEPDYJgHWRNg2vY9Q2188J4ModRp5Km/8pJdfk0nLmDj3yqbBO+gDtC3XqBUFx pTeMcEW7vGZvOKCQAst0mL0g0HY49DRZeHbv8leZlqcDeFSczvZM6qME5R5oryk+5kLZoi pO+cMi0KxC1//tTD9ZzwAiKoN5DkOjY= Received: from mail-qt1-f198.google.com (mail-qt1-f198.google.com [209.85.160.198]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-151-pJ1mqHxOMyyfA3XvUMwBdQ-1; Tue, 26 Oct 2021 12:08:34 -0400 X-MC-Unique: pJ1mqHxOMyyfA3XvUMwBdQ-1 Received: by mail-qt1-f198.google.com with SMTP id x28-20020ac8701c000000b0029f4b940566so8233536qtm.19 for ; Tue, 26 Oct 2021 09:08:34 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references:date :message-id:mime-version; bh=i4v4kvOLc6p73lzfkSEeP9f1UOcaSpfa5SPRiWCOs2k=; b=qEkjpZfOFC8fScrnCRRITt34/nNlc+bQW2ySGaxDo4N9Ji8MAIzlZYzCAr/QtrTKn7 xhKwnki2DArAluV3d/WJ130zThLG8qLSZFNcHVHWlf8m+apWk4/QpD0phKxir8JLK+aB OSoel9qT+qjTmQAVZDmzRP9SRrAk9o67m6PJb4APOV6tB5eo8dj/2gUQIxUS08HKlnlR 88btu15RlwxYfbMHDA3KBQfBeGlMmvdwCe1v2qg8TRePkcc6If/Onf8feGnLghCIjK3c YIh7WGUj+/nAh/R17chChqzwYFylpgXihGmAqDewYeWqnB7flU04aAHLuizvhUArNkqw DMrg== X-Gm-Message-State: AOAM530vlpwTuK6eXIFwtxNEFwjIeh4F9tyaX4RcnkPdpVC85R+Eb+V/ Jk4d09nrqYk6P9Ao8hVdTe3E+ku4yztMJa0LrrXPuLZBlXTbLXB0X3Zc4sbKfs+Vkx+Ci69UAV1 kVTwRIV/qqCI= X-Received: by 2002:a37:aac8:: with SMTP id t191mr20205305qke.52.1635264509744; Tue, 26 Oct 2021 09:08:29 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwo695JfmOfK1M6jtCRZO4kxw1CaVJb8yonjMYPBoBHAt3MVClcroc5VYWwgbuicAMO7ypySg== X-Received: by 2002:a37:aac8:: with SMTP id t191mr20205066qke.52.1635264507173; Tue, 26 Oct 2021 09:08:27 -0700 (PDT) Received: from localhost ([2601:184:4181:74c0:862e:5809:ed9e:e10e]) by smtp.gmail.com with ESMTPSA id n2sm11154963qtk.8.2021.10.26.09.08.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 26 Oct 2021 09:08:26 -0700 (PDT) From: Robbie Harwood To: Daniel Kiper Cc: grub-devel@gnu.org Subject: Re: [PATCH v3] Print module name on license check failure In-Reply-To: <20211026115251.v6wkvcuuk3kuwrpd@tomti.i.net-space.pl> References: <20211025221703.168221-1-rharwood@redhat.com> <20211026115251.v6wkvcuuk3kuwrpd@tomti.i.net-space.pl> Date: Tue, 26 Oct 2021 12:08:23 -0400 Message-ID: MIME-Version: 1.0 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=rharwood@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Received-SPF: pass client-ip=170.10.129.124; envelope-from=rharwood@redhat.com; helo=us-smtp-delivery-124.mimecast.com 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, DKIMWL_WL_HIGH=-0.001, 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-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Oct 2021 16:08:39 -0000 --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Daniel Kiper writes: > On Mon, Oct 25, 2021 at 06:17:03PM -0400, Robbie Harwood wrote: >> Before performing the license check, resolve the module name so that it >> can be printed if the license check fails. Prior to this change, grub >> would only indicate that the check had been failed, but not by what >> module. This made it difficult to track down either the problem module, >> or debug the false positive further. >> >> Signed-off-by: Robbie Harwood >> --- >> grub-core/kern/dl.c | 21 ++++++++++++++------- >> 1 file changed, 14 insertions(+), 7 deletions(-) >> >> diff --git a/grub-core/kern/dl.c b/grub-core/kern/dl.c >> index 48f8a7907..363bacc43 100644 >> --- a/grub-core/kern/dl.c >> +++ b/grub-core/kern/dl.c >> @@ -457,14 +457,21 @@ grub_dl_find_section (Elf_Ehdr *e, const char *nam= e) >> Be sure to understand your license obligations. >> */ >> static grub_err_t >> -grub_dl_check_license (Elf_Ehdr *e) >> +grub_dl_check_license (grub_dl_t mod, Elf_Ehdr *e) >> { >> Elf_Shdr *s =3D grub_dl_find_section (e, ".module_license"); >> - if (s && (grub_strcmp ((char *) e + s->sh_offset, "LICENSE=3DGPLv3") = =3D=3D 0 >> -=09 || grub_strcmp ((char *) e + s->sh_offset, "LICENSE=3DGPLv3+") = =3D=3D 0 >> -=09 || grub_strcmp ((char *) e + s->sh_offset, "LICENSE=3DGPLv2+") = =3D=3D 0)) >> + if (!s) > > s =3D=3D NULL please... > >> + return grub_error (GRUB_ERR_BAD_MODULE, >> +=09=09 "no license section in module %.64s", mod->name); >> + >> + if (grub_strcmp ((char *) e + s->sh_offset, "LICENSE=3DGPLv3") =3D=3D= 0 >> + || grub_strcmp ((char *) e + s->sh_offset, "LICENSE=3DGPLv3+") = =3D=3D 0 >> + || grub_strcmp ((char *) e + s->sh_offset, "LICENSE=3DGPLv2+") = =3D=3D 0) > > Could use grub_strncmp() instead of grub_strcmp() here? I don't object to doing so for clarity, but strictly speaking I don't think it's needed as the LICENSE=3D strings are terminated. >> return GRUB_ERR_NONE; >> - return grub_error (GRUB_ERR_BAD_MODULE, "incompatible license"); >> + >> + return grub_error (GRUB_ERR_BAD_MODULE, >> +=09=09 "incompatible license in module %.64s: %.64s", mod->name, >> +=09=09 (char *) e + s->sh_offset); >> } >> >> static grub_err_t >> @@ -641,8 +648,8 @@ grub_dl_load_core_noinit (void *addr, grub_size_t si= ze) >> constitutes linking) and GRUB core being licensed under GPLv3+. >> Be sure to understand your license obligations. >> */ >> - if (grub_dl_check_license (e) >> - || grub_dl_resolve_name (mod, e) >> + if (grub_dl_resolve_name (mod, e) >> + || grub_dl_check_license (mod, e) > > I think you should split this patch into two. One which improves error > messages and another which imposes length restrictions on the > grub_strcmp() and grub_error() in the grub_dl_check_license(). I can do that, but the NULL-check and strcmp are both pre-existing code from when the license check was added. Are you sure? > And I think you should use 63 instead of 64 (63 + NUL gives us 64 > :-)). Fine. Be well, --Robbie --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQJIBAEBCgAyFiEEA5qc6hnelQjDaHWqJTL5F2qVpEIFAmF4J/cUHHJoYXJ3b29k QHJlZGhhdC5jb20ACgkQJTL5F2qVpEJNJBAAgyiDrleDisJrSk9p3ZXRlQiHcyjz f8uNK1lLdCffsroMg4tXH7YCPPacYIsx126kriVwMHHZZc5BbEpPqSEU418Wc/yc k+mTSyU2liRrQ1BDTlajkM6aEE4XaWJG2MOSbg5jjQ0itrXjoS54DyNORbALIfq5 v+5Nxejwv+qToXbc71q9lnjD3TYpfypySYr6wDKVUS4wWAeaAN93C/R6j3YHy9/0 bieKWDmp7MvjWvCYYyNCee4kTBz8uiOt3GtsRSqxs7PXSHkqNi8lkNNPnABdiFi4 VSHNmsIzw9+bfpWs+WnGlDIjAJcTKDazFICUDcgW/MM0X3HCDLyW76gkh4Yx6lZ7 sv3Zzs+SlTpsOfhNOzq5UEBSAEEwBfD0wBknUXFjPcVHLH3lot+Ki5pGuFpy5Onx IxVVKNQwxSgRxAWc+uPiy9pUW759f6VWxHX5yGuF95ZSLxRNiOsXSZ9f2HrCsPvg 9SOQ9ODy7H5h6N+xRUNziif97EozQcFDpv8oIbACb8EZHTQTqJ3jEdXKgOUmA4Sp MNBJelr/zCHgdc4wVov5kcvvlS6NoLv6tFFFxHgj4tqH4IotYf+lPMXdSgOGlQ6o tiU24f554EjkggBCcF13G1jB1j1Migq+tUkaXs5mtuiAZBuZ++zYiXN3hOKplRxu 3tJqlGoC9s3hzUo= =hKa7 -----END PGP SIGNATURE----- --=-=-=--