linux-kernel - Re: [PATCH v4 08/35] iommu/mediatek: Use kmalloc for protect buffer

Open Source and information security mailing list archives

Date:   Wed, 16 Feb 2022 16:11:42 +0800
From:   Yong Wu <yong.wu@...iatek.com>
To:     Tomasz Figa <tfiga@...omium.org>
CC:     AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>,
        Robin Murphy <robin.murphy@....com>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>,
        <linux-mediatek@...ts.infradead.org>,
        <srv_heupstream@...iatek.com>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <iommu@...ts.linux-foundation.org>,
        Hsin-Yi Wang <hsinyi@...omium.org>, <youlin.pei@...iatek.com>,
        <anan.sun@...iatek.com>, <xueqi.zhang@...iatek.com>,
        <yen-chang.chen@...iatek.com>, <mingyuan.ma@...iatek.com>,
        <yf.wang@...iatek.com>, <libo.kang@...iatek.com>,
        <chengci.xu@...iatek.com>, Joerg Roedel <joro@...tes.org>,
        Rob Herring <robh+dt@...nel.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        "Will Deacon" <will@...nel.org>
Subject: Re: [PATCH v4 08/35] iommu/mediatek: Use kmalloc for protect buffer

On Wed, 2022-02-16 at 14:59 +0900, Tomasz Figa wrote:
> On Wed, Feb 16, 2022 at 2:55 PM Yong Wu <yong.wu@...iatek.com> wrote:
> > 
> > On Thu, 2022-01-27 at 12:08 +0100, AngeloGioacchino Del Regno
> > wrote:
> > > Il 25/01/22 09:56, Yong Wu ha scritto:
> > > > No need zero for the protect buffer that is only accessed by
> > > > the
> > > > IOMMU HW
> > > > translation fault happened.
> > > > 
> > > > Signed-off-by: Yong Wu <yong.wu@...iatek.com>
> > > 
> > > I would rather keep this a devm_kzalloc instead... the cost is
> > > very
> > > minimal and
> > > this will be handy when new hardware will be introduced, as it
> > > may
> > > require a bigger
> > > buffer: in that case, "older" platforms will use only part of it
> > > and
> > > we may get
> > > garbage data at the end.
> > 
> > Currently this is to avoid zero 512 bytes for all the platforms.
> > 
> > Sorry, I don't understand why it is unnecessary when the new
> > hardware
> > requires a bigger buffer. If the buffer becomes bigger, then
> > clearing
> > it to 0 need more cost. then this patch is more helpful?
> > 
> > The content in this buffer is garbage, we won't care about or
> > analyse
> > it.
> 
> I think we should zero it for security reasons regardless of any
> other
> aspects. With this patch it's leaking kernel data to the hardware.
> 
> At the same time, we're talking here about something executed just 1
> time when the driver probes. I don't think the cost would really
> matter.

OK. I will remove this patch in next version.

Thanks.

> 
> Best regards,
> Tomasz
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

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