Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 15 Sep 2022 16:49:57 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Herbert Xu <herbert@gondor.apana.org.au>
Cc:     George Cherian <gcherian@marvell.com>,
        "David S. Miller" <davem@davemloft.net>,
        David Daney <david.daney@cavium.com>,
        linux-crypto@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: Re: [PATCH] crypto: cavium - prevent integer overflow loading
 firmware
Message-ID: <YyMthStaEzAStvS1@kadam>
References: <YxDQpc9IINUuUhQr@kili>
 <Yxm5HrtXBiz6gKtv@gondor.apana.org.au>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Yxm5HrtXBiz6gKtv@gondor.apana.org.au>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?kl4jFzQEtzmD0RcP4Uh7paZ8YtWtu8PyT0q96kCiepDPoVRGLUWM/srtk06F?=
 =?us-ascii?Q?tTGoB2vDTe3gkjZ4xhZGN5hKOHQKz5SoKa9jPIoLTnAlFOqVOlv4eUeQUWVM?=
 =?us-ascii?Q?81SR/mvAjoNWQPVeLQJ1QUGJSh6y1qgQuPEVimVdK+eK7MQuhk1kSywtgQ02?=
 =?us-ascii?Q?QV0u4kFYVOj8XqDLfenN4nMAFdChd1sr8XiHGCi5hRQbiOShOrKe+azwvl4A?=
 =?us-ascii?Q?4M03oBWhzfjQgBAFS3snoxGlDEA5luynaYlEs6ypv5CnGw346k+TckALDAlR?=
 =?us-ascii?Q?RJLoSkkmtTefrmdr1N8R/x1OAVvb7B5AieLHVKoS/jIfB4ZAyMtspu8Uu38l?=
 =?us-ascii?Q?F1rSx+aYdrYtM1D/eVnozrqAUXUeF/M0lxoUp/iqJhdGj0n85IEG2AtlgvLV?=
 =?us-ascii?Q?h2ejxxv4DAIDR7w6kehL/jA4ogcGpVd3w6uiwp5V8rMjsFMCK4XY8lhhz5TJ?=
 =?us-ascii?Q?xxxKdmJ6VHBYwELVp1uczwHp0A3+n/XR4AtZ8DAKaLhQIF23SsHbV5bEsi+k?=
 =?us-ascii?Q?ZdOYlM4IbiIn+4vKXYqWFAdCraPkl3feoYQgZBIeWtuuLrffd7qY6av0+6yU?=
 =?us-ascii?Q?G/aXghY/ro2PYVNFLnOJ8n9yQH6Q/jGgxd/8RoGDiFPMPQsA/bsZjQ9N9KqY?=
 =?us-ascii?Q?Thq/NPWxmibmzbDgOtRPZ2LikCUEknfrYj/tEtmSNT7+XGfTLegtEiyTNgEt?=
 =?us-ascii?Q?DaUw9I5dmHmwXa+3qcuLskVmyFVC5+Iq8mYN5dq+kuwJU9IxOhEvL0+cilmC?=
 =?us-ascii?Q?5wwkHRwGf7DArP1nlBFBF96XP/v5CPx/vTqncDmfWiCLWS2rhtb8URq0XO2w?=
 =?us-ascii?Q?7qq8K17QJafArdbsduwpIwh4izLoWIQ2BTtB8A3aljILn2zty8az2dYStsjD?=
 =?us-ascii?Q?h3V68gN47HNC8//yJYeLUXFLVU/0vubnMzAFkbnFbQ4tbhlCOnmaBJ3xpsaz?=
 =?us-ascii?Q?NFnRLwuf2PW3SHOirzdVAMWlRvdTCAFX2/MI61uG/up40X/ahIKc2D3b12K3?=
 =?us-ascii?Q?2mkpz72vRPJmN57TImZMVE7A++HdinsOg/aXTxJCR70FpP1+v4OCwylwxAnb?=
 =?us-ascii?Q?ZpeJqya24rj53gOZ7ZMCJj8bQBJ3qHZ0VBKjKENvlOiTdXYknkTY7Li/3cGi?=
 =?us-ascii?Q?7KCdMhOX/EAFByiz7NLFggDBSREYic9jzXsz/UO7uRVeDb6s+yXW0VUraxkY?=
 =?us-ascii?Q?n0LR8AbzgdEG9xOqM/sQRpef8HTDVaytgTAKurZV82yuq8FtcEYU+/tfJxLL?=
 =?us-ascii?Q?OgwEPZ1Y8hSyOyJH/Bgo6pV5V1jiekALZ3k7iFYLUqHDmpL183l7G6JbRyLp?=
 =?us-ascii?Q?2aq00SZa+Dwh31M6UlKPZSg576t1EvTKrIxVxsBB9kshcJ2MQ8uMXAmO3mus?=
 =?us-ascii?Q?WRNEnd5vNmQI+klA0ZxQXBh+EWjoVYb7+ei3MK5MqhAcLrueuT1om2jSkDdI?=
 =?us-ascii?Q?AjChtlz4YtLcNGoLhIwq7Kuju02QKbrJp/TVhtY53bIiJfSjW3I4PhSckRGF?=
 =?us-ascii?Q?BfZ6woloQIuTaI/8AxdGL3WLsV/5GcLWtV788N5Tgk6vUV9/Bkn+bpLrY91N?=
 =?us-ascii?Q?UJd9btXu7T3UXaB5XDs8H0EAbgTqvNXL9IpLI3V7ZtL+fGjLQuXVh2tvhqwO?=
 =?us-ascii?Q?GA=3D=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 66bd2d26-ba24-4347-a094-08da972136cc
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Sep 2022 13:50:14.0753
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: r+SKMp89QbyK4WZFO9LqMFqsY7rKje9G78QP+h5Mgq+a7IhWTEiDD/9dVUmI4sHup/b1EpA8GZTqsu+F0xDIdQCrfmhFjtzrob4/F/fmIbs=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLAPR10MB5124
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.528,FMLib:17.11.122.1
 definitions=2022-09-15_08,2022-09-14_04,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 mlxscore=0
 malwarescore=0 spamscore=0 mlxlogscore=999 adultscore=0 phishscore=0
 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2208220000 definitions=main-2209150080
X-Proofpoint-ORIG-GUID: ba8lnmIm5TZwit0DMejz5zuGZByLl3zF
X-Proofpoint-GUID: ba8lnmIm5TZwit0DMejz5zuGZByLl3zF
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org

On Thu, Sep 08, 2022 at 05:42:54PM +0800, Herbert Xu wrote:
> On Thu, Sep 01, 2022 at 06:32:53PM +0300, Dan Carpenter wrote:
> >
> > @@ -263,7 +264,13 @@ static int cpt_ucode_load_fw(struct cpt_device *cpt, const u8 *fw, bool is_ae)
> >  	ucode = (struct ucode_header *)fw_entry->data;
> >  	mcode = &cpt->mcode[cpt->next_mc_idx];
> >  	memcpy(mcode->version, (u8 *)fw_entry->data, CPT_UCODE_VERSION_SZ);
> > -	mcode->code_size = ntohl(ucode->code_length) * 2;
> > +
> > +	code_length = ntohl(ucode->code_length);
> > +	if (code_length >= INT_MAX / 2) {
> > +		ret = -EINVAL;
> > +		goto fw_release;
> > +	}
> > +	mcode->code_size = code_length;
> 
> Where did the "* 2" go?

Crud.  :/  Sorry.

> 
> BTW, what is the threat model here? If the firmware metadata can't
> be trusted, shouldn't we be capping the firmware size at a level
> a lot lower than INT_MAX?

This is not firmware metadata, I'm fairly sure the fw_entry->data is raw
data from the file system.  Realistically, if you can't trust your
firmware then you are probably toasted but there is a move to trust as
little as possible.  Also Smatch marks data from the file system as
untrusted so it will generate static checker warnings.

regards,
dan carpenter