linux-kernel - Re: [PATCH 1/2] media: ir-kbd-i2c: prevent potential NULL pointer access

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190725075538.GB1323@kunai>
Date:   Thu, 25 Jul 2019 09:55:38 +0200
From:   Wolfram Sang <wsa@...-dreams.de>
To:     Sean Young <sean@...s.org>
Cc:     Wolfram Sang <wsa+renesas@...g-engineering.com>,
        linux-i2c@...r.kernel.org,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] media: ir-kbd-i2c: prevent potential NULL pointer
 access

Hi Sean,

thanks for the review!

On Thu, Jul 25, 2019 at 06:12:02AM +0100, Sean Young wrote:
> On Mon, Jul 22, 2019 at 07:26:31PM +0200, Wolfram Sang wrote:
> > i2c_new_dummy() can fail returning a NULL pointer. The code does not
> > bail out in this case and the returned pointer is blindly used.
> 
> I don't see how. The existing code tries to set up the tx part; if
> i2c_new_dummy() return NULL then the rcdev is registered without tx,
> and tx_c is never used.

Yes, you are totally right. I missed that the send_block function is
also only called iff zilog_init succeeded. Thanks for the heads up and
sorry for the noise.

> 
> > Convert
> > to devm_i2c_new_dummy_device() which returns an ERR_PTR and also bail
> > out when failing the validity check.
> 
> Possibly I was being overly cautious with not bailing out if tx can't
> be registered; moving to devm is probably a good idea. However the
> commit message is misleading, because the existing code has no
> NULL pointer access.

Yep, I will resend with a proper commit message. Technically, there is
no need to bail out anymore because there is no NULL pointer access. My
tendency is now to not bail out and keep the old behaviour (registering
without tx). What do you think?

Regards,

   Wolfram


Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)