On 2/11/21 11:22 AM, Andy Shevchenko wrote:
On Thu, Feb 11, 2021 at 12:04:11AM +0100, Maximilian Luz wrote:
The raw message frame length is unaligned and explicitly marked as
little endian. It should not be accessed without the appropriatte
accessor functions. Fix this.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Though a few nit-picks below.
Reported-by: kernel-test-robot <lkp@xxxxxxxxx>
Fixes: c167b9c7e3d6 ("platform/surface: Add Surface Aggregator subsystem")
Signed-off-by: Maximilian Luz <luzmaximilian@xxxxxxxxx>
---
drivers/platform/surface/aggregator/ssh_packet_layer.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/surface/aggregator/ssh_packet_layer.c b/drivers/platform/surface/aggregator/ssh_packet_layer.c
index 583315db8b02..9a78188d8d1c 100644
--- a/drivers/platform/surface/aggregator/ssh_packet_layer.c
+++ b/drivers/platform/surface/aggregator/ssh_packet_layer.c
@@ -1774,7 +1774,8 @@ static size_t ssh_ptl_rx_eval(struct ssh_ptl *ptl, struct ssam_span *source)
break;
}
- return aligned.ptr - source->ptr + SSH_MESSAGE_LENGTH(frame->len);
+ return aligned.ptr - source->ptr
+ + SSH_MESSAGE_LENGTH(get_unaligned_le16(&frame->len));
I would leave + on previous line.
I can fix that if it bugs you.
Also it's possible to annotate temporary variable and use it, but it seems not
worth to do.
Now that you mention it, we already have the correct frame length in
payload.len. Let me draft up a new patch with that.
Side question: Do you think the below is correct (& operator)?
sp.len = get_unaligned_le16(&((struct ssh_frame *)sf.ptr)->len);
To me seems like you take an address to len member rather its value.
That's the point though, no? The signature is
u16 get_unaligned_le16(const void *p)
so we do want a pointer to the len member. So I believe that is correct.
}
static int ssh_ptl_rx_threadfn(void *data)
