Skip to content

Commit d375b35

Browse files
Suzuki K Poulosegregkh
Suzuki K Poulose
authored andcommitted
coresight: Fix support for sparsely populated ports
On some systems the firmware may not describe all the ports connected to a component (e.g, for security reasons). This could be especially problematic for "funnels" where we could end up in modifying memory beyond the allocated space for refcounts. e.g, for a funnel with input ports listed 0, 3, 5, nr_inport = 3. However the we could access refcnts[5] while checking for references, like : [ 526.110401] ================================================================== [ 526.117988] BUG: KASAN: slab-out-of-bounds in funnel_enable+0x54/0x1b0 [ 526.124706] Read of size 4 at addr ffffff8135f9549c by task bash/1114 [ 526.131324] [ 526.132886] CPU: 3 PID: 1114 Comm: bash Tainted: G S 5.4.25 #232 [ 526.140397] Hardware name: Qualcomm Technologies, Inc. SC7180 IDP (DT) [ 526.147113] Call trace: [ 526.149653] dump_backtrace+0x0/0x188 [ 526.153431] show_stack+0x20/0x2c [ 526.156852] dump_stack+0xdc/0x144 [ 526.160370] print_address_description+0x3c/0x494 [ 526.165211] __kasan_report+0x144/0x168 [ 526.169170] kasan_report+0x10/0x18 [ 526.172769] check_memory_region+0x1a4/0x1b4 [ 526.177164] __kasan_check_read+0x18/0x24 [ 526.181292] funnel_enable+0x54/0x1b0 [ 526.185072] coresight_enable_path+0x104/0x198 [ 526.189649] coresight_enable+0x118/0x26c ... [ 526.237782] Allocated by task 280: [ 526.241298] __kasan_kmalloc+0xf0/0x1ac [ 526.245249] kasan_kmalloc+0xc/0x14 [ 526.248849] __kmalloc+0x28c/0x3b4 [ 526.252361] coresight_register+0x88/0x250 [ 526.256587] funnel_probe+0x15c/0x228 [ 526.260365] dynamic_funnel_probe+0x20/0x2c [ 526.264679] amba_probe+0xbc/0x158 [ 526.268193] really_probe+0x144/0x408 [ 526.271970] driver_probe_device+0x70/0x140 ... [ 526.316810] [ 526.318364] Freed by task 0: [ 526.321344] (stack is not available) [ 526.325024] [ 526.326580] The buggy address belongs to the object at ffffff8135f95480 [ 526.326580] which belongs to the cache kmalloc-128 of size 128 [ 526.339439] The buggy address is located 28 bytes inside of [ 526.339439] 128-byte region [ffffff8135f95480, ffffff8135f95500) [ 526.351399] The buggy address belongs to the page: [ 526.356342] page:ffffffff04b7e500 refcount:1 mapcount:0 mapping:ffffff814b00c380 index:0x0 compound_mapcount: 0 [ 526.366711] flags: 0x4000000000010200(slab|head) [ 526.371475] raw: 4000000000010200 ffffffff05034008 ffffffff0501eb08 ffffff814b00c380 [ 526.379435] raw: 0000000000000000 0000000000190019 00000001ffffffff 0000000000000000 [ 526.387393] page dumped because: kasan: bad access detected [ 526.393128] [ 526.394681] Memory state around the buggy address: [ 526.399619] ffffff8135f95380: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc [ 526.407046] ffffff8135f95400: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc [ 526.414473] >ffffff8135f95480: 04 fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc [ 526.421900] ^ [ 526.426029] ffffff8135f95500: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc [ 526.433456] ffffff8135f95580: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc [ 526.440883] ================================================================== To keep the code simple, we now track the maximum number of possible input/output connections to/from this component @ nr_inport and nr_outport in platform_data, respectively. Thus the output connections could be sparse and code is adjusted to skip the unspecified connections. Cc: Mathieu Poirier <[email protected]> Cc: Mike Leach <[email protected]> Reported-by: Sai Prakash Ranjan <[email protected]> Tested-by: Sai Prakash Ranjan <[email protected]> Tested-by: Stephen Boyd <[email protected]> Signed-off-by: Suzuki K Poulose <[email protected]> Signed-off-by: Mathieu Poirier <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 1c33c65 commit d375b35

File tree

3 files changed

+72
-30
lines changed

3 files changed

+72
-30
lines changed

drivers/hwtracing/coresight/coresight-platform.c

Lines changed: 60 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -87,17 +87,24 @@ static void of_coresight_get_ports_legacy(const struct device_node *node,
8787
int *nr_inport, int *nr_outport)
8888
{
8989
struct device_node *ep = NULL;
90+
struct of_endpoint endpoint;
9091
int in = 0, out = 0;
9192

9293
do {
9394
ep = of_graph_get_next_endpoint(node, ep);
9495
if (!ep)
9596
break;
9697

97-
if (of_coresight_legacy_ep_is_input(ep))
98-
in++;
99-
else
100-
out++;
98+
if (of_graph_parse_endpoint(ep, &endpoint))
99+
continue;
100+
101+
if (of_coresight_legacy_ep_is_input(ep)) {
102+
in = (endpoint.port + 1 > in) ?
103+
endpoint.port + 1 : in;
104+
} else {
105+
out = (endpoint.port + 1) > out ?
106+
endpoint.port + 1 : out;
107+
}
101108

102109
} while (ep);
103110

@@ -137,9 +144,16 @@ of_coresight_count_ports(struct device_node *port_parent)
137144
{
138145
int i = 0;
139146
struct device_node *ep = NULL;
147+
struct of_endpoint endpoint;
148+
149+
while ((ep = of_graph_get_next_endpoint(port_parent, ep))) {
150+
/* Defer error handling to parsing */
151+
if (of_graph_parse_endpoint(ep, &endpoint))
152+
continue;
153+
if (endpoint.port + 1 > i)
154+
i = endpoint.port + 1;
155+
}
140156

141-
while ((ep = of_graph_get_next_endpoint(port_parent, ep)))
142-
i++;
143157
return i;
144158
}
145159

@@ -191,21 +205,20 @@ static int of_coresight_get_cpu(struct device *dev)
191205
* Parses the local port, remote device name and the remote port.
192206
*
193207
* Returns :
194-
* 1 - If the parsing is successful and a connection record
195-
* was created for an output connection.
196208
* 0 - If the parsing completed without any fatal errors.
197209
* -Errno - Fatal error, abort the scanning.
198210
*/
199211
static int of_coresight_parse_endpoint(struct device *dev,
200212
struct device_node *ep,
201-
struct coresight_connection *conn)
213+
struct coresight_platform_data *pdata)
202214
{
203215
int ret = 0;
204216
struct of_endpoint endpoint, rendpoint;
205217
struct device_node *rparent = NULL;
206218
struct device_node *rep = NULL;
207219
struct device *rdev = NULL;
208220
struct fwnode_handle *rdev_fwnode;
221+
struct coresight_connection *conn;
209222

210223
do {
211224
/* Parse the local port details */
@@ -232,6 +245,13 @@ static int of_coresight_parse_endpoint(struct device *dev,
232245
break;
233246
}
234247

248+
conn = &pdata->conns[endpoint.port];
249+
if (conn->child_fwnode) {
250+
dev_warn(dev, "Duplicate output port %d\n",
251+
endpoint.port);
252+
ret = -EINVAL;
253+
break;
254+
}
235255
conn->outport = endpoint.port;
236256
/*
237257
* Hold the refcount to the target device. This could be
@@ -244,7 +264,6 @@ static int of_coresight_parse_endpoint(struct device *dev,
244264
conn->child_fwnode = fwnode_handle_get(rdev_fwnode);
245265
conn->child_port = rendpoint.port;
246266
/* Connection record updated */
247-
ret = 1;
248267
} while (0);
249268

250269
of_node_put(rparent);
@@ -258,7 +277,6 @@ static int of_get_coresight_platform_data(struct device *dev,
258277
struct coresight_platform_data *pdata)
259278
{
260279
int ret = 0;
261-
struct coresight_connection *conn;
262280
struct device_node *ep = NULL;
263281
const struct device_node *parent = NULL;
264282
bool legacy_binding = false;
@@ -287,8 +305,6 @@ static int of_get_coresight_platform_data(struct device *dev,
287305
dev_warn_once(dev, "Uses obsolete Coresight DT bindings\n");
288306
}
289307

290-
conn = pdata->conns;
291-
292308
/* Iterate through each output port to discover topology */
293309
while ((ep = of_graph_get_next_endpoint(parent, ep))) {
294310
/*
@@ -300,15 +316,9 @@ static int of_get_coresight_platform_data(struct device *dev,
300316
if (legacy_binding && of_coresight_legacy_ep_is_input(ep))
301317
continue;
302318

303-
ret = of_coresight_parse_endpoint(dev, ep, conn);
304-
switch (ret) {
305-
case 1:
306-
conn++; /* Fall through */
307-
case 0:
308-
break;
309-
default:
319+
ret = of_coresight_parse_endpoint(dev, ep, pdata);
320+
if (ret)
310321
return ret;
311-
}
312322
}
313323

314324
return 0;
@@ -647,6 +657,16 @@ static int acpi_coresight_parse_link(struct acpi_device *adev,
647657
* coresight_remove_match().
648658
*/
649659
conn->child_fwnode = fwnode_handle_get(&r_adev->fwnode);
660+
} else if (dir == ACPI_CORESIGHT_LINK_SLAVE) {
661+
/*
662+
* We are only interested in the port number
663+
* for the input ports at this component.
664+
* Store the port number in child_port.
665+
*/
666+
conn->child_port = fields[0].integer.value;
667+
} else {
668+
/* Invalid direction */
669+
return -EINVAL;
650670
}
651671

652672
return dir;
@@ -692,10 +712,20 @@ static int acpi_coresight_parse_graph(struct acpi_device *adev,
692712
return dir;
693713

694714
if (dir == ACPI_CORESIGHT_LINK_MASTER) {
695-
pdata->nr_outport++;
715+
if (ptr->outport > pdata->nr_outport)
716+
pdata->nr_outport = ptr->outport;
696717
ptr++;
697718
} else {
698-
pdata->nr_inport++;
719+
WARN_ON(pdata->nr_inport == ptr->child_port);
720+
/*
721+
* We do not track input port connections for a device.
722+
* However we need the highest port number described,
723+
* which can be recorded now and reuse this connection
724+
* record for an output connection. Hence, do not move
725+
* the ptr for input connections
726+
*/
727+
if (ptr->child_port > pdata->nr_inport)
728+
pdata->nr_inport = ptr->child_port;
699729
}
700730
}
701731

@@ -704,8 +734,13 @@ static int acpi_coresight_parse_graph(struct acpi_device *adev,
704734
return rc;
705735

706736
/* Copy the connection information to the final location */
707-
for (i = 0; i < pdata->nr_outport; i++)
708-
pdata->conns[i] = conns[i];
737+
for (i = 0; conns + i < ptr; i++) {
738+
int port = conns[i].outport;
739+
740+
/* Duplicate output port */
741+
WARN_ON(pdata->conns[port].child_fwnode);
742+
pdata->conns[port] = conns[i];
743+
}
709744

710745
devm_kfree(&adev->dev, conns);
711746
return 0;

drivers/hwtracing/coresight/coresight.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1053,6 +1053,9 @@ static int coresight_orphan_match(struct device *dev, void *data)
10531053
for (i = 0; i < i_csdev->pdata->nr_outport; i++) {
10541054
conn = &i_csdev->pdata->conns[i];
10551055

1056+
/* Skip the port if FW doesn't describe it */
1057+
if (!conn->child_fwnode)
1058+
continue;
10561059
/* We have found at least one orphan connection */
10571060
if (conn->child_dev == NULL) {
10581061
/* Does it match this newly added device? */
@@ -1091,6 +1094,8 @@ static int coresight_fixup_device_conns(struct coresight_device *csdev)
10911094
for (i = 0; i < csdev->pdata->nr_outport; i++) {
10921095
struct coresight_connection *conn = &csdev->pdata->conns[i];
10931096

1097+
if (!conn->child_fwnode)
1098+
continue;
10941099
conn->child_dev =
10951100
coresight_find_csdev_by_fwnode(conn->child_fwnode);
10961101
if (conn->child_dev) {
@@ -1126,7 +1131,7 @@ static int coresight_remove_match(struct device *dev, void *data)
11261131
for (i = 0; i < iterator->pdata->nr_outport; i++) {
11271132
conn = &iterator->pdata->conns[i];
11281133

1129-
if (conn->child_dev == NULL)
1134+
if (conn->child_dev == NULL || conn->child_fwnode == NULL)
11301135
continue;
11311136

11321137
if (csdev->dev.fwnode == conn->child_fwnode) {

include/linux/coresight.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,12 @@ union coresight_dev_subtype {
100100
};
101101

102102
/**
103-
* struct coresight_platform_data - data harvested from the DT specification
104-
* @nr_inport: number of input ports for this component.
105-
* @nr_outport: number of output ports for this component.
106-
* @conns: Array of nr_outport connections from this component
103+
* struct coresight_platform_data - data harvested from the firmware
104+
* specification.
105+
*
106+
* @nr_inport: Number of elements for the input connections.
107+
* @nr_outport: Number of elements for the output connections.
108+
* @conns: Sparse array of nr_outport connections from this component.
107109
*/
108110
struct coresight_platform_data {
109111
int nr_inport;

0 commit comments

Comments
 (0)