Commit b50a24e9 authored by Greg Kroah-Hartman's avatar Greg Kroah-Hartman

greybus: svc: don't trust any struct devices

As the svc code is "odd" in how things are set up at initial connection
time, we can't always "know" that we have a valid bundle to use for
error messages.  So just punt and always use pr_*() calls to ensure that
we never incorrectly dereference a pointer.

This will get fixed up soon, but for now, let's just live with a bit
messier error messages in the log.
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@google.com>
Reviewed-by: default avatarViresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: default avatarAlex Elder <elder@linaro.org>
Reviewed-by: default avatarJohan Hovold <johan@hovoldconsulting.com>
parent c69b98d1
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
* Released under the GPLv2 only. * Released under the GPLv2 only.
*/ */
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <linux/workqueue.h> #include <linux/workqueue.h>
#include "greybus.h" #include "greybus.h"
...@@ -120,17 +121,15 @@ int gb_svc_dme_peer_get(struct gb_svc *svc, u8 intf_id, u16 attr, u16 selector, ...@@ -120,17 +121,15 @@ int gb_svc_dme_peer_get(struct gb_svc *svc, u8 intf_id, u16 attr, u16 selector,
&request, sizeof(request), &request, sizeof(request),
&response, sizeof(response)); &response, sizeof(response));
if (ret) { if (ret) {
dev_err(&svc->connection->dev, pr_err("failed to get DME attribute (%hhu %hx %hu) %d\n",
"failed to get DME attribute (%hhu %hx %hu) %d\n", intf_id, attr, selector, ret);
intf_id, attr, selector, ret);
return ret; return ret;
} }
result = le16_to_cpu(response.result_code); result = le16_to_cpu(response.result_code);
if (result) { if (result) {
dev_err(&svc->connection->dev, pr_err("Unipro error %hu while getting DME attribute (%hhu %hx %hu)\n",
"Unipro error %hu while getting DME attribute (%hhu %hx %hu)\n", result, intf_id, attr, selector);
result, intf_id, attr, selector);
return -EINVAL; return -EINVAL;
} }
...@@ -158,17 +157,15 @@ int gb_svc_dme_peer_set(struct gb_svc *svc, u8 intf_id, u16 attr, u16 selector, ...@@ -158,17 +157,15 @@ int gb_svc_dme_peer_set(struct gb_svc *svc, u8 intf_id, u16 attr, u16 selector,
&request, sizeof(request), &request, sizeof(request),
&response, sizeof(response)); &response, sizeof(response));
if (ret) { if (ret) {
dev_err(&svc->connection->dev, pr_err("failed to set DME attribute (%hhu %hx %hu %u) %d\n",
"failed to set DME attribute (%hhu %hx %hu %u) %d\n", intf_id, attr, selector, value, ret);
intf_id, attr, selector, value, ret);
return ret; return ret;
} }
result = le16_to_cpu(response.result_code); result = le16_to_cpu(response.result_code);
if (result) { if (result) {
dev_err(&svc->connection->dev, pr_err("Unipro error %hu while setting DME attribute (%hhu %hx %hu %u)\n",
"Unipro error %hu while setting DME attribute (%hhu %hx %hu %u)\n", result, intf_id, attr, selector, value);
result, intf_id, attr, selector, value);
return -EINVAL; return -EINVAL;
} }
...@@ -270,11 +267,9 @@ void gb_svc_connection_destroy(struct gb_svc *svc, u8 intf1_id, u16 cport1_id, ...@@ -270,11 +267,9 @@ void gb_svc_connection_destroy(struct gb_svc *svc, u8 intf1_id, u16 cport1_id,
ret = gb_operation_sync(connection, GB_SVC_TYPE_CONN_DESTROY, ret = gb_operation_sync(connection, GB_SVC_TYPE_CONN_DESTROY,
&request, sizeof(request), NULL, 0); &request, sizeof(request), NULL, 0);
if (ret) { if (ret)
dev_err(&connection->dev, pr_err("failed to destroy connection (%hhx:%hx %hhx:%hx) %d\n",
"failed to destroy connection (%hhx:%hx %hhx:%hx) %d\n", intf1_id, cport1_id, intf2_id, cport2_id, ret);
intf1_id, cport1_id, intf2_id, cport2_id, ret);
}
} }
EXPORT_SYMBOL_GPL(gb_svc_connection_destroy); EXPORT_SYMBOL_GPL(gb_svc_connection_destroy);
...@@ -304,11 +299,9 @@ static void gb_svc_route_destroy(struct gb_svc *svc, u8 intf1_id, u8 intf2_id) ...@@ -304,11 +299,9 @@ static void gb_svc_route_destroy(struct gb_svc *svc, u8 intf1_id, u8 intf2_id)
ret = gb_operation_sync(svc->connection, GB_SVC_TYPE_ROUTE_DESTROY, ret = gb_operation_sync(svc->connection, GB_SVC_TYPE_ROUTE_DESTROY,
&request, sizeof(request), NULL, 0); &request, sizeof(request), NULL, 0);
if (ret) { if (ret)
dev_err(&svc->connection->dev, pr_err("failed to destroy route (%hhx %hhx) %d\n",
"failed to destroy route (%hhx %hhx) %d\n",
intf1_id, intf2_id, ret); intf1_id, intf2_id, ret);
}
} }
static int gb_svc_version_request(struct gb_operation *op) static int gb_svc_version_request(struct gb_operation *op)
...@@ -316,14 +309,13 @@ static int gb_svc_version_request(struct gb_operation *op) ...@@ -316,14 +309,13 @@ static int gb_svc_version_request(struct gb_operation *op)
struct gb_connection *connection = op->connection; struct gb_connection *connection = op->connection;
struct gb_protocol_version_request *request; struct gb_protocol_version_request *request;
struct gb_protocol_version_response *response; struct gb_protocol_version_response *response;
struct device *dev = &connection->dev;
request = op->request->payload; request = op->request->payload;
if (request->major > GB_SVC_VERSION_MAJOR) { if (request->major > GB_SVC_VERSION_MAJOR) {
dev_err(&connection->dev, pr_err("%d: unsupported major version (%hhu > %hhu)\n",
"unsupported major version (%hhu > %hhu)\n", connection->intf_cport_id, request->major,
request->major, GB_SVC_VERSION_MAJOR); GB_SVC_VERSION_MAJOR);
return -ENOTSUPP; return -ENOTSUPP;
} }
...@@ -331,8 +323,8 @@ static int gb_svc_version_request(struct gb_operation *op) ...@@ -331,8 +323,8 @@ static int gb_svc_version_request(struct gb_operation *op)
connection->module_minor = request->minor; connection->module_minor = request->minor;
if (!gb_operation_response_alloc(op, sizeof(*response), GFP_KERNEL)) { if (!gb_operation_response_alloc(op, sizeof(*response), GFP_KERNEL)) {
dev_err(dev, "%s: error allocating response\n", pr_err("%d: error allocating response\n",
__func__); connection->intf_cport_id);
return -ENOMEM; return -ENOMEM;
} }
...@@ -348,7 +340,6 @@ static int gb_svc_hello(struct gb_operation *op) ...@@ -348,7 +340,6 @@ static int gb_svc_hello(struct gb_operation *op)
struct gb_connection *connection = op->connection; struct gb_connection *connection = op->connection;
struct greybus_host_device *hd = connection->hd; struct greybus_host_device *hd = connection->hd;
struct gb_svc_hello_request *hello_request; struct gb_svc_hello_request *hello_request;
struct device *dev = &connection->dev;
struct gb_interface *intf; struct gb_interface *intf;
u16 endo_id; u16 endo_id;
u8 interface_id; u8 interface_id;
...@@ -359,9 +350,9 @@ static int gb_svc_hello(struct gb_operation *op) ...@@ -359,9 +350,9 @@ static int gb_svc_hello(struct gb_operation *op)
* request, use that to create an endo. * request, use that to create an endo.
*/ */
if (op->request->payload_size < sizeof(*hello_request)) { if (op->request->payload_size < sizeof(*hello_request)) {
dev_err(dev, "%s: Illegal size of hello request (%zu < %zu)\n", pr_err("%d: Illegal size of hello request (%zu < %zu)\n",
__func__, op->request->payload_size, connection->intf_cport_id, op->request->payload_size,
sizeof(*hello_request)); sizeof(*hello_request));
return -EINVAL; return -EINVAL;
} }
...@@ -418,7 +409,6 @@ static void svc_process_hotplug(struct work_struct *work) ...@@ -418,7 +409,6 @@ static void svc_process_hotplug(struct work_struct *work)
struct gb_connection *connection = svc_hotplug->connection; struct gb_connection *connection = svc_hotplug->connection;
struct gb_svc *svc = connection->private; struct gb_svc *svc = connection->private;
struct greybus_host_device *hd = connection->hd; struct greybus_host_device *hd = connection->hd;
struct device *dev = &connection->dev;
struct gb_interface *intf; struct gb_interface *intf;
u8 intf_id, device_id; u8 intf_id, device_id;
int ret; int ret;
...@@ -444,15 +434,15 @@ static void svc_process_hotplug(struct work_struct *work) ...@@ -444,15 +434,15 @@ static void svc_process_hotplug(struct work_struct *work)
* Remove the interface and add it again, and let user know * Remove the interface and add it again, and let user know
* about this with a print message. * about this with a print message.
*/ */
dev_info(dev, "Removed interface (%hhu) to add it again\n", pr_info("%d: Removed interface (%hhu) to add it again\n",
intf_id); connection->intf_cport_id, intf_id);
svc_intf_remove(connection, intf); svc_intf_remove(connection, intf);
} }
intf = gb_interface_create(hd, intf_id); intf = gb_interface_create(hd, intf_id);
if (!intf) { if (!intf) {
dev_err(dev, "%s: Failed to create interface with id %hhu\n", pr_err("%d: Failed to create interface with id %hhu\n",
__func__, intf_id); connection->intf_cport_id, intf_id);
goto free_svc_hotplug; goto free_svc_hotplug;
} }
...@@ -477,15 +467,15 @@ static void svc_process_hotplug(struct work_struct *work) ...@@ -477,15 +467,15 @@ static void svc_process_hotplug(struct work_struct *work)
GB_DEVICE_ID_MODULES_START, 0, GFP_KERNEL); GB_DEVICE_ID_MODULES_START, 0, GFP_KERNEL);
if (device_id < 0) { if (device_id < 0) {
ret = device_id; ret = device_id;
dev_err(dev, "%s: Failed to allocate device id for interface with id %hhu (%d)\n", pr_err("%d: Failed to allocate device id for interface with id %hhu (%d)\n",
__func__, intf_id, ret); connection->intf_cport_id, intf_id, ret);
goto destroy_interface; goto destroy_interface;
} }
ret = gb_svc_intf_device_id(svc, intf_id, device_id); ret = gb_svc_intf_device_id(svc, intf_id, device_id);
if (ret) { if (ret) {
dev_err(dev, "%s: Device id operation failed, interface %hhu device_id %hhu (%d)\n", pr_err("%d: Device id operation failed, interface %hhu device_id %hhu (%d)\n",
__func__, intf_id, device_id, ret); connection->intf_cport_id, intf_id, device_id, ret);
goto ida_put; goto ida_put;
} }
...@@ -495,15 +485,15 @@ static void svc_process_hotplug(struct work_struct *work) ...@@ -495,15 +485,15 @@ static void svc_process_hotplug(struct work_struct *work)
ret = gb_svc_route_create(svc, hd->endo->ap_intf_id, GB_DEVICE_ID_AP, ret = gb_svc_route_create(svc, hd->endo->ap_intf_id, GB_DEVICE_ID_AP,
intf_id, device_id); intf_id, device_id);
if (ret) { if (ret) {
dev_err(dev, "%s: Route create operation failed, interface %hhu device_id %hhu (%d)\n", pr_err("%d: Route create operation failed, interface %hhu device_id %hhu (%d)\n",
__func__, intf_id, device_id, ret); connection->intf_cport_id, intf_id, device_id, ret);
goto svc_id_free; goto svc_id_free;
} }
ret = gb_interface_init(intf, device_id); ret = gb_interface_init(intf, device_id);
if (ret) { if (ret) {
dev_err(dev, "%s: Failed to initialize interface, interface %hhu device_id %hhu (%d)\n", pr_err("%d: Failed to initialize interface, interface %hhu device_id %hhu (%d)\n",
__func__, intf_id, device_id, ret); connection->intf_cport_id, intf_id, device_id, ret);
goto destroy_route; goto destroy_route;
} }
...@@ -539,10 +529,9 @@ static int gb_svc_intf_hotplug_recv(struct gb_operation *op) ...@@ -539,10 +529,9 @@ static int gb_svc_intf_hotplug_recv(struct gb_operation *op)
struct svc_hotplug *svc_hotplug; struct svc_hotplug *svc_hotplug;
if (request->payload_size < sizeof(svc_hotplug->data)) { if (request->payload_size < sizeof(svc_hotplug->data)) {
dev_err(&op->connection->dev, pr_err("%d: short hotplug request received (%zu < %zu)\n",
"%s: short hotplug request received (%zu < %zu)\n", op->connection->intf_cport_id, request->payload_size,
__func__, request->payload_size, sizeof(svc_hotplug->data));
sizeof(svc_hotplug->data));
return -EINVAL; return -EINVAL;
} }
...@@ -564,13 +553,13 @@ static int gb_svc_intf_hot_unplug_recv(struct gb_operation *op) ...@@ -564,13 +553,13 @@ static int gb_svc_intf_hot_unplug_recv(struct gb_operation *op)
struct gb_message *request = op->request; struct gb_message *request = op->request;
struct gb_svc_intf_hot_unplug_request *hot_unplug = request->payload; struct gb_svc_intf_hot_unplug_request *hot_unplug = request->payload;
struct greybus_host_device *hd = op->connection->hd; struct greybus_host_device *hd = op->connection->hd;
struct device *dev = &op->connection->dev;
struct gb_interface *intf; struct gb_interface *intf;
u8 intf_id; u8 intf_id;
if (request->payload_size < sizeof(*hot_unplug)) { if (request->payload_size < sizeof(*hot_unplug)) {
dev_err(dev, "short hot unplug request received (%zu < %zu)\n", pr_err("connection %d: short hot unplug request received (%zu < %zu)\n",
request->payload_size, sizeof(*hot_unplug)); op->connection->intf_cport_id, request->payload_size,
sizeof(*hot_unplug));
return -EINVAL; return -EINVAL;
} }
...@@ -578,8 +567,8 @@ static int gb_svc_intf_hot_unplug_recv(struct gb_operation *op) ...@@ -578,8 +567,8 @@ static int gb_svc_intf_hot_unplug_recv(struct gb_operation *op)
intf = gb_interface_find(hd, intf_id); intf = gb_interface_find(hd, intf_id);
if (!intf) { if (!intf) {
dev_err(dev, "%s: Couldn't find interface for id %hhu\n", pr_err("connection %d: Couldn't find interface for id %hhu\n",
__func__, intf_id); op->connection->intf_cport_id, intf_id);
return -EINVAL; return -EINVAL;
} }
...@@ -595,9 +584,9 @@ static int gb_svc_intf_reset_recv(struct gb_operation *op) ...@@ -595,9 +584,9 @@ static int gb_svc_intf_reset_recv(struct gb_operation *op)
u8 intf_id; u8 intf_id;
if (request->payload_size < sizeof(*reset)) { if (request->payload_size < sizeof(*reset)) {
dev_err(&op->connection->dev, pr_err("connection %d: short reset request received (%zu < %zu)\n",
"short reset request received (%zu < %zu)\n", op->connection->intf_cport_id, request->payload_size,
request->payload_size, sizeof(*reset)); sizeof(*reset));
return -EINVAL; return -EINVAL;
} }
reset = request->payload; reset = request->payload;
...@@ -641,9 +630,8 @@ static int gb_svc_request_recv(u8 type, struct gb_operation *op) ...@@ -641,9 +630,8 @@ static int gb_svc_request_recv(u8 type, struct gb_operation *op)
} }
if (ret) { if (ret) {
dev_warn(&connection->dev, pr_warn("connection %d: unexpected SVC request 0x%02x received (state %u)\n",
"unexpected SVC request 0x%02x received (state %u)\n", connection->intf_cport_id, type, svc->state);
type, svc->state);
return ret; return ret;
} }
...@@ -665,8 +653,8 @@ static int gb_svc_request_recv(u8 type, struct gb_operation *op) ...@@ -665,8 +653,8 @@ static int gb_svc_request_recv(u8 type, struct gb_operation *op)
case GB_SVC_TYPE_INTF_RESET: case GB_SVC_TYPE_INTF_RESET:
return gb_svc_intf_reset_recv(op); return gb_svc_intf_reset_recv(op);
default: default:
dev_err(&op->connection->dev, pr_err("connection %d: unsupported request: %hhu\n",
"unsupported request: %hhu\n", type); connection->intf_cport_id, type);
return -EINVAL; return -EINVAL;
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment