Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
From:   Yusong Gao <a869920004@gmail.com>
To:     jarkko@kernel.org
Cc:     a869920004@gmail.com, davem@davemloft.net, dhowells@redhat.com,
        dimitri.ledkov@canonical.com, dwmw2@infradead.org,
        herbert@gondor.apana.org.au, juergh@proton.me,
        keyrings@vger.kernel.org, linux-crypto@vger.kernel.org,
        linux-kernel@vger.kernel.org, lists@sapience.com,
        zohar@linux.ibm.com
Subject: [PATCH v3] sign-file: Fix incorrect return values check
Date:   Wed, 22 Nov 2023 03:19:22 +0000
Message-Id: <20231122031922.40983-1-a869920004@gmail.com>
In-Reply-To: <89f0dbe3a390dd2d49b09c0ff75a142aa44793fd.camel@kernel.org>
References: <89f0dbe3a390dd2d49b09c0ff75a142aa44793fd.camel@kernel.org>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Nov 22, 2023 at 4:54 AM Jarkko Sakkinen <jarkko@kernel.org>=0D
wrote:=0D
>=0D
> On Tue, 2023-11-21 at 03:40 +0000, Yusong Gao wrote:=0D
> > There are some wrong return values check in sign-file when call=0D
> > OpenSSL=0D
> > API. The ERR() check cond is wrong because of the program only check=0D
> > the=0D
> > return value is < 0 instead of <=3D 0. For example:=0D
> > 1. CMS_final() return 1 for success or 0 for failure.=0D
> > 2. i2d_CMS_bio_stream() returns 1 for success or 0 for failure.=0D
> > 3. i2d_TYPEbio() return 1 for success and 0 for failure.=0D
> > 4. BIO_free() return 1 for success and 0 for failure.=0D
> >=0D
> > Link: https://www.openssl.org/docs/manmaster/man3/=0D
> > Fixes: e5a2e3c84782 ("scripts/sign-file.c: Add support for signing=0D
> > with a raw signature")=0D
> >=0D
>=0D
> No empty line here.=0D
>=0D
> > Signed-off-by: Yusong Gao <a869920004@gmail.com>=0D
> > ---=0D
> > V1, V2: Clarify the description of git message.=0D
> > ---=0D
> >  scripts/sign-file.c | 12 ++++++------=0D
> >  1 file changed, 6 insertions(+), 6 deletions(-)=0D
> >=0D
> > diff --git a/scripts/sign-file.c b/scripts/sign-file.c=0D
> > index 598ef5465f82..dcebbcd6bebd 100644=0D
> > --- a/scripts/sign-file.c=0D
> > +++ b/scripts/sign-file.c=0D
> > @@ -322,7 +322,7 @@ int main(int argc, char **argv)=0D
> >                                    CMS_NOSMIMECAP | use_keyid |=0D
> >                                    use_signed_attrs),=0D
> >                   "CMS_add1_signer");=0D
> > -             ERR(CMS_final(cms, bm, NULL, CMS_NOCERTS |=0D
> > CMS_BINARY) < 0,=0D
> > +             ERR(CMS_final(cms, bm, NULL, CMS_NOCERTS |=0D
> > CMS_BINARY) <=3D 0,=0D
> >                   "CMS_final");=0D
> >  =0D
> >  #else=0D
> > @@ -341,10 +341,10 @@ int main(int argc, char **argv)=0D
> >                       b =3D BIO_new_file(sig_file_name, "wb");=0D
> >                       ERR(!b, "%s", sig_file_name);=0D
> >  #ifndef USE_PKCS7=0D
> > -                     ERR(i2d_CMS_bio_stream(b, cms, NULL, 0) < 0,=0D
> > +                     ERR(i2d_CMS_bio_stream(b, cms, NULL, 0) <=3D=0D
> > 0,=0D
> >                           "%s", sig_file_name);=0D
> >  #else=0D
> > -                     ERR(i2d_PKCS7_bio(b, pkcs7) < 0,=0D
> > +                     ERR(i2d_PKCS7_bio(b, pkcs7) <=3D 0,=0D
> >                           "%s", sig_file_name);=0D
> >  #endif=0D
> >                       BIO_free(b);=0D
> > @@ -374,9 +374,9 @@ int main(int argc, char **argv)=0D
> >  =0D
> >       if (!raw_sig) {=0D
> >  #ifndef USE_PKCS7=0D
> > -             ERR(i2d_CMS_bio_stream(bd, cms, NULL, 0) < 0, "%s",=0D
> > dest_name);=0D
> > +             ERR(i2d_CMS_bio_stream(bd, cms, NULL, 0) <=3D 0, "%s",=0D
> > dest_name);=0D
> >  #else=0D
> > -             ERR(i2d_PKCS7_bio(bd, pkcs7) < 0, "%s", dest_name);=0D
> > +             ERR(i2d_PKCS7_bio(bd, pkcs7) <=3D 0, "%s", dest_name);=0D
> >  #endif=0D
> >       } else {=0D
> >               BIO *b;=0D
> > @@ -396,7 +396,7 @@ int main(int argc, char **argv)=0D
> >       ERR(BIO_write(bd, &sig_info, sizeof(sig_info)) < 0, "%s",=0D
> > dest_name);=0D
> >       ERR(BIO_write(bd, magic_number, sizeof(magic_number) - 1) <=0D
> > 0, "%s", dest_name);=0D
> >  =0D
> > -     ERR(BIO_free(bd) < 0, "%s", dest_name);=0D
> > +     ERR(BIO_free(bd) <=3D 0, "%s", dest_name);=0D
> >  =0D
> >       /* Finally, if we're signing in place, replace the original.=0D
> > */=0D
> >       if (replace_orig)=0D
>=0D
>=0D
> BR, Jarkko=0D
=0D
Thanks.=0D
=0D
BR, Yusong Gao=0D