[PATCH 1/1] api: add missing cookie checks for network access
dev_write_net() and dev_read_net() should validate the provided cookie.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- api/api_net.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/api/api_net.c b/api/api_net.c index 7515c26e8b4..0b931a80787 100644 --- a/api/api_net.c +++ b/api/api_net.c @@ -72,14 +72,16 @@ int dev_enum_net(struct device_info *di)
int dev_write_net(void *cookie, void *buf, int len) { - /* XXX verify that cookie points to a valid net device??? */ + if (!dev_valid_net(cookie)) + return API_ENODEV;
return eth_send(buf, len); }
int dev_read_net(void *cookie, void *buf, int len) { - /* XXX verify that cookie points to a valid net device??? */ + if (!dev_valid_net(cookie)) + return API_ENODEV;
return eth_receive(buf, len); }
On Tue, Apr 09, 2024 at 03:16:55PM +0200, Heinrich Schuchardt wrote:
dev_write_net() and dev_read_net() should validate the provided cookie.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
api/api_net.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/api/api_net.c b/api/api_net.c index 7515c26e8b4..0b931a80787 100644 --- a/api/api_net.c +++ b/api/api_net.c @@ -72,14 +72,16 @@ int dev_enum_net(struct device_info *di)
int dev_write_net(void *cookie, void *buf, int len) {
- /* XXX verify that cookie points to a valid net device??? */
if (!dev_valid_net(cookie))
return API_ENODEV;return eth_send(buf, len);
}
int dev_read_net(void *cookie, void *buf, int len) {
- /* XXX verify that cookie points to a valid net device??? */
if (!dev_valid_net(cookie))
return API_ENODEV;return eth_receive(buf, len);
}
Is this right? Probably. But what triggered looking in to this to start with? I don't think anything is enabling the API support these days even.
On 4/10/24 02:43, Tom Rini wrote:
On Tue, Apr 09, 2024 at 03:16:55PM +0200, Heinrich Schuchardt wrote:
dev_write_net() and dev_read_net() should validate the provided cookie.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
api/api_net.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/api/api_net.c b/api/api_net.c index 7515c26e8b4..0b931a80787 100644 --- a/api/api_net.c +++ b/api/api_net.c @@ -72,14 +72,16 @@ int dev_enum_net(struct device_info *di)
int dev_write_net(void *cookie, void *buf, int len) {
- /* XXX verify that cookie points to a valid net device??? */
if (!dev_valid_net(cookie))
return API_ENODEV;return eth_send(buf, len); }
int dev_read_net(void *cookie, void *buf, int len) {
- /* XXX verify that cookie points to a valid net device??? */
if (!dev_valid_net(cookie))
return API_ENODEV;return eth_receive(buf, len); }
Is this right? Probably. But what triggered looking in to this to start with? I don't think anything is enabling the API support these days even.
We should either properly test the API in our CI or or remove it.
What once was done via the API could be done via an EFI payload in a more portable way today.
Best regards
Heinrich
On Wed, Apr 10, 2024 at 07:13:21AM +0200, Heinrich Schuchardt wrote:
On 4/10/24 02:43, Tom Rini wrote:
On Tue, Apr 09, 2024 at 03:16:55PM +0200, Heinrich Schuchardt wrote:
dev_write_net() and dev_read_net() should validate the provided cookie.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
api/api_net.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/api/api_net.c b/api/api_net.c index 7515c26e8b4..0b931a80787 100644 --- a/api/api_net.c +++ b/api/api_net.c @@ -72,14 +72,16 @@ int dev_enum_net(struct device_info *di) int dev_write_net(void *cookie, void *buf, int len) {
- /* XXX verify that cookie points to a valid net device??? */
- if (!dev_valid_net(cookie))
return API_ENODEV;
- /* XXX verify that cookie points to a valid net device??? */
- if (!dev_valid_net(cookie))
return API_ENODEV;Is this right? Probably. But what triggered looking in to this to start with? I don't think anything is enabling the API support these days even.
We should either properly test the API in our CI or or remove it.
What once was done via the API could be done via an EFI payload in a more portable way today.
Yes, we should indeed likely remove it.
participants (2)
-
Heinrich Schuchardt -
Tom Rini