From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by mx.groups.io with SMTP id smtpd.web10.2813.1670535605433801223 for ; Thu, 08 Dec 2022 13:40:05 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=G17q3Olk; spf=pass (domain: kernel.org, ip: 145.40.68.75, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 1405AB8264D for ; Thu, 8 Dec 2022 21:40:03 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6307BC43398 for ; Thu, 8 Dec 2022 21:40:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1670535602; bh=je6J910+Zs6/+rrY3fDgFE+VTFGgySWptHeqkLsn7MM=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=G17q3OlkbvcU8hV7ubvw3+euubnYDfqCOHcnXXiZa/SiTBCvUYeA75V1B3nqomqht YY2CuiFU++w0WnBr+VmJNO/JmLe4i9thewlrwyXaNIeGCJWnLC3fP2/neeMozpINIO KYZ7guhVwk/elQ/8XcalXaMYn/HVBAT21ccZugKcVmHMQrKFbK7cDMYJWc+h+7zsev +1BKXneaIB2rhLv+LOWFbrysGMiQ9k1YxrTKzC0hIK/oF+yHdsUN1RpzakHBJlGoUQ 9zTXmlw09F/g0MezqXdKcS7gxBysMx5MrK4zngtjwpmJ1y9+uPWcnbJSqx6wejf14+ NAfVIGo5rU4uA== Received: by mail-lj1-f181.google.com with SMTP id b9so2987738ljr.5 for ; Thu, 08 Dec 2022 13:40:02 -0800 (PST) X-Gm-Message-State: ANoB5pmRnB12fwfNWNSkUZHc7PVmL2IEvLwHmZByUfWPqzFG7X8qdNGY Jx1TlJMTzMmu8uWdLV3IvNJAyMONilYr6kSK21w= X-Google-Smtp-Source: AA0mqf4i2XtsDBFhqzQAptgXer8DJYopvBwI1zlBsxkcJ3QlroocZgEQe5kahdehydTd84aYLC25IbEjrubZQagwG8E= X-Received: by 2002:a05:651c:1603:b0:26d:d603:8df2 with SMTP id f3-20020a05651c160300b0026dd6038df2mr27466790ljq.189.1670535600359; Thu, 08 Dec 2022 13:40:00 -0800 (PST) MIME-Version: 1.0 References: <20221207161245.2554193-1-ardb@kernel.org> In-Reply-To: From: "Ard Biesheuvel" Date: Thu, 8 Dec 2022 22:39:49 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH] ShellPkg: Avoid corrupting installed device path protocols To: devel@edk2.groups.io, michael.d.kinney@intel.com Cc: "Ni, Ray" , "Gao, Zhichao" Content-Type: text/plain; charset="UTF-8" On Thu, 8 Dec 2022 at 22:15, Michael D Kinney wrote: > > Hi Ard, > > There is a difference between returning a pointer to a device path > and modifying the device path contents. > > If you add CONST to the argument, then an updated pointer to a device > path can not be returned. > No, this is incorrect. The function takes a pointer (1) to a pointer(2) to a device path protocol EFI_DEVICE_PATH_PROTOCOL ** So the function can dereference pointer 1 and modify pointer 2 *unless* it is marked as CONST, i.e. EFI_DEVICE_PATH_PROTOCOL * CONST * in which case the pointer is not modifiable, but it is permitted to dereference that pointer to modify the underlying object. I am arguing that the prototype should be EFI_DEVICE_PATH_PROTOCOL CONST ** (which is the same as putting the CONST at the beginning) where the caller's pointer can be advanced by the callee via the pointer-to-pointer. But that would still not permit the object to be modified. > The API clear describes returning an updated device path pointer, so > the API is declared correctly without CONST. > The pointer may be updated but not the object. It really comes down to the difference between CONST EFI_DEVICE_PATH_PROTOCOL ** EFI_DEVICE_PATH_PROTOCOL CONST ** EFI_DEVICE_PATH_PROTOCOL * CONST * EFI_DEVICE_PATH_PROTOCOL **CONST (where the first two mean the same thing0 > The API does not state that the contents of the device path are modified. > > An API that uses CONST EFI_DEVICE_PATH* would indicate that the API > should not modify the contents of the device path. For example: > > /** > Returns the size of a device path in bytes. > > This function returns the size, in bytes, of the device path data structure > specified by DevicePath including the end of device path node. > If DevicePath is NULL or invalid, then 0 is returned. > > @param DevicePath A pointer to a device path data structure. > > @retval 0 If DevicePath is NULL or invalid. > @retval Others The size of a device path in bytes. > > **/ > UINTN > EFIAPI > GetDevicePathSize ( > IN CONST EFI_DEVICE_PATH_PROTOCOL *DevicePath > ); > Yes, but this one is a pointer, not a pointer-to-pointer. Big difference.