linux-kernel - Re: [PATCH] drm/amd/amdgpu: Properly tune the size of struct

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: <da6c860f-0ef0-44e4-8b58-0f4d55c1b8bd@kadam.mountain>
Date:   Tue, 20 Jun 2023 10:37:59 +0300
From:   Dan Carpenter <dan.carpenter@...aro.org>
To:     Su Hui <suhui@...china.com>
Cc:     alexander.deucher@....com, airlied@...il.com, Xinhui.Pan@....com,
        daniel@...ll.ch, David.Francis@....com, Jane.Jian@....com,
        Bokun.Zhang@....com, monk.liu@....com,
        amd-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org
Subject: Re: [PATCH] drm/amd/amdgpu: Properly tune the size of struct

On Tue, Jun 20, 2023 at 12:59:19PM +0800, Su Hui wrote:
> Smatch error:
>     gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:316:49: error:
>     static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
>     static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
> 

I doubt that moving the struct members around is safe.  This looks like
it's in a very specific order.  So I don't think this patch is correct.

The reason for this false positive this code uses a #pragma to pack the
struct.

#pragma pack(push, 1) // PF2VF / VF2PF data areas are byte packed

Sparse does not support this and Smatch uses Sparse as a parser.  The
main reason why Sparse doesn't support this pragma is because Linus
thinks it's gross.  You probably didn't even see the #pragma did you?
And anything you can't see is unreadable by definition.

"Mark the associated types properly packed individually, rather than
use the disgusting "pragma pack()" that should never be used."

https://lore.kernel.org/linux-sparse/CAHk-=wi7jGZ+bVbt-UfXOkpEQdHzF3Z2HBjkGdjh8q4dvPPGWQ@mail.gmail.com/

regards,
dan carpenter