* [PATCH 0/3] staging: vchiq: Fix vchiq_read return value in case of error @ 2019-11-20 15:36 Marcelo Diop-Gonzalez 2019-11-20 15:36 ` [PATCH 1/3] staging: vchiq_dump: Replace min with min_t Marcelo Diop-Gonzalez ` (4 more replies) 0 siblings, 5 replies; 16+ messages in thread From: Marcelo Diop-Gonzalez @ 2019-11-20 15:36 UTC (permalink / raw) To: gregkh, eric, wahrenst; +Cc: devel, linux-rpi-kernel This is a proposed fix of an issue regarding the handling of positive return values from copy_to_user() in vchiq_read(). It looks like the struct dump_context passed to the vchiq_dump() function keeps track of the number of bytes written in the context->actual field. When copy_to_user() returns a positive value, this is set to -EFAULT. The problem is that this is never returned to the user, and instead the function continues, adding the number of bytes that should have been copied to context->actual. Running the following program: #include <stdio.h> #include <stdlib.h> #include <fcntl.h> #include <unistd.h> #include <errno.h> int main(int argc, char **argv) { int fd = open("/dev/vchiq", 0); if (fd < 0) exit(-1); sbrk(getpagesize()); char *bad_buf = sbrk(0)-100; int x = read(fd, bad_buf, 2000); printf("%d %d\n", x, errno); puts(bad_buf); } gives this output: -1 1 State 0: CONNECTED tx_po Remote: slots 2-32 tx_pos=578 recycle=1f Slots claimed: Note the mangled output and incorrect errno value. Messing with the constants in that toy program changes the results. Sometimes read() returns with no error, sometimes it returns with a wrong error code, sometimes with the right one. But it seems that it only ever returns an error at all accidentally, due to the fact that the comparison between context->actual and context->space in vchiq_dump() is unsigned, so that that function won't do anything else if it ever sets context->actual to a negative value. After this patchset, the above program prints this: -1 14 State 0: CONNECTED tx_pos=b4a218(@165de6b4), rx_pos=ae0668(@f02b54f4) Version: 8 (min 3) Stats Help with testing would be appreciated. So far I've basically just diffed the output of 'cat /dev/vchiq', run the program above with a few different values, and run vchiq_test a few times. These were applied to the staging-next branch of the tree at git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git Marcelo Diop-Gonzalez (3): staging: vchiq_dump: Replace min with min_t staging: vchiq: Refactor indentation in vchiq_dump_* functions staging: vchiq: Have vchiq_dump_* functions return an error code .../interface/vchiq_arm/vchiq_2835_arm.c | 5 +- .../interface/vchiq_arm/vchiq_arm.c | 129 ++++++++++-------- .../interface/vchiq_arm/vchiq_core.c | 104 ++++++++++---- .../interface/vchiq_arm/vchiq_core.h | 12 +- 4 files changed, 153 insertions(+), 97 deletions(-) -- 2.20.1 _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3] staging: vchiq_dump: Replace min with min_t 2019-11-20 15:36 [PATCH 0/3] staging: vchiq: Fix vchiq_read return value in case of error Marcelo Diop-Gonzalez @ 2019-11-20 15:36 ` Marcelo Diop-Gonzalez 2019-11-20 15:46 ` Greg KH 2019-11-20 15:36 ` [PATCH 2/3] staging: vchiq: Refactor indentation in vchiq_dump_* functions Marcelo Diop-Gonzalez ` (3 subsequent siblings) 4 siblings, 1 reply; 16+ messages in thread From: Marcelo Diop-Gonzalez @ 2019-11-20 15:36 UTC (permalink / raw) To: gregkh, eric, wahrenst; +Cc: devel, linux-rpi-kernel Replacing this and fixing the block comment format in this function fixes checkpatch warnings. Signed-off-by: Marcelo Diop-Gonzalez <marcgonzalez@google.com> --- .../vc04_services/interface/vchiq_arm/vchiq_arm.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index 319e7aa0e0e0..942b4768c88e 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -2081,7 +2081,7 @@ vchiq_dump(void *dump_context, const char *str, int len) int copy_bytes; if (context->offset > 0) { - int skip_bytes = min(len, (int)context->offset); + int skip_bytes = min_t(int, len, context->offset); str += skip_bytes; len -= skip_bytes; @@ -2089,7 +2089,7 @@ vchiq_dump(void *dump_context, const char *str, int len) if (context->offset > 0) return; } - copy_bytes = min(len, (int)(context->space - context->actual)); + copy_bytes = min_t(int, len, context->space - context->actual); if (copy_bytes == 0) return; if (copy_to_user(context->buf + context->actual, str, @@ -2098,9 +2098,11 @@ vchiq_dump(void *dump_context, const char *str, int len) context->actual += copy_bytes; len -= copy_bytes; - /* If tne terminating NUL is included in the length, then it - ** marks the end of a line and should be replaced with a - ** carriage return. */ + /* + * If the terminating NUL is included in the length, then it + * marks the end of a line and should be replaced with a + * carriage return. + */ if ((len == 0) && (str[copy_bytes - 1] == '\0')) { char cr = '\n'; -- 2.20.1 _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] staging: vchiq_dump: Replace min with min_t 2019-11-20 15:36 ` [PATCH 1/3] staging: vchiq_dump: Replace min with min_t Marcelo Diop-Gonzalez @ 2019-11-20 15:46 ` Greg KH 0 siblings, 0 replies; 16+ messages in thread From: Greg KH @ 2019-11-20 15:46 UTC (permalink / raw) To: Marcelo Diop-Gonzalez; +Cc: devel, eric, wahrenst, linux-rpi-kernel On Wed, Nov 20, 2019 at 10:36:46AM -0500, Marcelo Diop-Gonzalez wrote: > Replacing this and fixing the block comment format in this > function fixes checkpatch warnings. That is two different things. Which means you need two different patches here. Please fix up. thanks, greg k-h _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] staging: vchiq: Refactor indentation in vchiq_dump_* functions 2019-11-20 15:36 [PATCH 0/3] staging: vchiq: Fix vchiq_read return value in case of error Marcelo Diop-Gonzalez 2019-11-20 15:36 ` [PATCH 1/3] staging: vchiq_dump: Replace min with min_t Marcelo Diop-Gonzalez @ 2019-11-20 15:36 ` Marcelo Diop-Gonzalez 2019-11-20 15:36 ` [PATCH 3/3] staging: vchiq: Have vchiq_dump_* functions return an error code Marcelo Diop-Gonzalez ` (2 subsequent siblings) 4 siblings, 0 replies; 16+ messages in thread From: Marcelo Diop-Gonzalez @ 2019-11-20 15:36 UTC (permalink / raw) To: gregkh, eric, wahrenst; +Cc: devel, linux-rpi-kernel Doing this helps with readability, and makes the logic easier to follow. Signed-off-by: Marcelo Diop-Gonzalez <marcgonzalez@google.com> --- .../interface/vchiq_arm/vchiq_arm.c | 104 +++++++++--------- 1 file changed, 53 insertions(+), 51 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index 942b4768c88e..8f9cfa083264 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -2076,40 +2076,40 @@ void vchiq_dump(void *dump_context, const char *str, int len) { struct dump_context *context = (struct dump_context *)dump_context; + int copy_bytes; - if (context->actual < context->space) { - int copy_bytes; + if (context->actual >= context->space) + return; - if (context->offset > 0) { - int skip_bytes = min_t(int, len, context->offset); + if (context->offset > 0) { + int skip_bytes = min_t(int, len, context->offset); - str += skip_bytes; - len -= skip_bytes; - context->offset -= skip_bytes; - if (context->offset > 0) - return; - } - copy_bytes = min_t(int, len, context->space - context->actual); - if (copy_bytes == 0) + str += skip_bytes; + len -= skip_bytes; + context->offset -= skip_bytes; + if (context->offset > 0) return; - if (copy_to_user(context->buf + context->actual, str, - copy_bytes)) + } + copy_bytes = min_t(int, len, context->space - context->actual); + if (copy_bytes == 0) + return; + if (copy_to_user(context->buf + context->actual, str, + copy_bytes)) + context->actual = -EFAULT; + context->actual += copy_bytes; + len -= copy_bytes; + + /* + * If the terminating NUL is included in the length, then it + * marks the end of a line and should be replaced with a + * carriage return. + */ + if ((len == 0) && (str[copy_bytes - 1] == '\0')) { + char cr = '\n'; + + if (copy_to_user(context->buf + context->actual - 1, + &cr, 1)) context->actual = -EFAULT; - context->actual += copy_bytes; - len -= copy_bytes; - - /* - * If the terminating NUL is included in the length, then it - * marks the end of a line and should be replaced with a - * carriage return. - */ - if ((len == 0) && (str[copy_bytes - 1] == '\0')) { - char cr = '\n'; - - if (copy_to_user(context->buf + context->actual - 1, - &cr, 1)) - context->actual = -EFAULT; - } } } @@ -2134,34 +2134,36 @@ vchiq_dump_platform_instances(void *dump_context) struct vchiq_service *service = state->services[i]; struct vchiq_instance *instance; - if (service && (service->base.callback == service_callback)) { - instance = service->instance; - if (instance) - instance->mark = 0; - } + if (!service || service->base.callback != service_callback) + continue; + + instance = service->instance; + if (instance) + instance->mark = 0; } for (i = 0; i < state->unused_service; i++) { struct vchiq_service *service = state->services[i]; struct vchiq_instance *instance; - if (service && (service->base.callback == service_callback)) { - instance = service->instance; - if (instance && !instance->mark) { - len = snprintf(buf, sizeof(buf), - "Instance %pK: pid %d,%s completions %d/%d", - instance, instance->pid, - instance->connected ? " connected, " : - "", - instance->completion_insert - - instance->completion_remove, - MAX_COMPLETIONS); - - vchiq_dump(dump_context, buf, len + 1); - - instance->mark = 1; - } - } + if (!service || service->base.callback != service_callback) + continue; + + instance = service->instance; + if (!instance || instance->mark) + continue; + + len = snprintf(buf, sizeof(buf), + "Instance %pK: pid %d,%s completions %d/%d", + instance, instance->pid, + instance->connected ? " connected, " : + "", + instance->completion_insert - + instance->completion_remove, + MAX_COMPLETIONS); + + vchiq_dump(dump_context, buf, len + 1); + instance->mark = 1; } } -- 2.20.1 _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/3] staging: vchiq: Have vchiq_dump_* functions return an error code 2019-11-20 15:36 [PATCH 0/3] staging: vchiq: Fix vchiq_read return value in case of error Marcelo Diop-Gonzalez 2019-11-20 15:36 ` [PATCH 1/3] staging: vchiq_dump: Replace min with min_t Marcelo Diop-Gonzalez 2019-11-20 15:36 ` [PATCH 2/3] staging: vchiq: Refactor indentation in vchiq_dump_* functions Marcelo Diop-Gonzalez @ 2019-11-20 15:36 ` Marcelo Diop-Gonzalez 2019-11-20 18:50 ` Dan Carpenter 2019-11-20 20:20 ` [PATCH v2 0/4] staging: vchiq: Fix vchiq_read return value in case of error Marcelo Diop-Gonzalez 2019-11-21 10:42 ` [PATCH 0/3] " Stefan Wahren 4 siblings, 1 reply; 16+ messages in thread From: Marcelo Diop-Gonzalez @ 2019-11-20 15:36 UTC (permalink / raw) To: gregkh, eric, wahrenst; +Cc: devel, linux-rpi-kernel These functions currently modify the struct dump_context passed to them, and set context->actual to -EFAULT in case of error. The issue is that this is never returned to the user (except accidentally when things align so that that happens). So, have these functions return 0 on success and the appropriate error code otherwise, and return nonzero errors to the user. Signed-off-by: Marcelo Diop-Gonzalez <marcgonzalez@google.com> --- .../interface/vchiq_arm/vchiq_2835_arm.c | 5 +- .../interface/vchiq_arm/vchiq_arm.c | 35 +++--- .../interface/vchiq_arm/vchiq_core.c | 104 +++++++++++++----- .../interface/vchiq_arm/vchiq_core.h | 12 +- 4 files changed, 104 insertions(+), 52 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c index e568e9e6eb95..ca30bfd52919 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c @@ -247,15 +247,14 @@ vchiq_complete_bulk(struct vchiq_bulk *bulk) bulk->actual); } -void -vchiq_dump_platform_state(void *dump_context) +int vchiq_dump_platform_state(void *dump_context) { char buf[80]; int len; len = snprintf(buf, sizeof(buf), " Platform: 2835 (VC master)"); - vchiq_dump(dump_context, buf, len + 1); + return vchiq_dump(dump_context, buf, len + 1); } enum vchiq_status diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index 8f9cfa083264..02148a24818a 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -2072,14 +2072,13 @@ static int vchiq_release(struct inode *inode, struct file *file) * ***************************************************************************/ -void -vchiq_dump(void *dump_context, const char *str, int len) +int vchiq_dump(void *dump_context, const char *str, int len) { struct dump_context *context = (struct dump_context *)dump_context; int copy_bytes; if (context->actual >= context->space) - return; + return 0; if (context->offset > 0) { int skip_bytes = min_t(int, len, context->offset); @@ -2088,14 +2087,14 @@ vchiq_dump(void *dump_context, const char *str, int len) len -= skip_bytes; context->offset -= skip_bytes; if (context->offset > 0) - return; + return 0; } copy_bytes = min_t(int, len, context->space - context->actual); if (copy_bytes == 0) - return; + return 0; if (copy_to_user(context->buf + context->actual, str, copy_bytes)) - context->actual = -EFAULT; + return -EFAULT; context->actual += copy_bytes; len -= copy_bytes; @@ -2109,8 +2108,9 @@ vchiq_dump(void *dump_context, const char *str, int len) if (copy_to_user(context->buf + context->actual - 1, &cr, 1)) - context->actual = -EFAULT; + return -EFAULT; } + return 0; } /**************************************************************************** @@ -2119,8 +2119,7 @@ vchiq_dump(void *dump_context, const char *str, int len) * ***************************************************************************/ -void -vchiq_dump_platform_instances(void *dump_context) +int vchiq_dump_platform_instances(void *dump_context) { struct vchiq_state *state = vchiq_get_state(); char buf[80]; @@ -2145,6 +2144,7 @@ vchiq_dump_platform_instances(void *dump_context) for (i = 0; i < state->unused_service; i++) { struct vchiq_service *service = state->services[i]; struct vchiq_instance *instance; + int err; if (!service || service->base.callback != service_callback) continue; @@ -2162,9 +2162,12 @@ vchiq_dump_platform_instances(void *dump_context) instance->completion_remove, MAX_COMPLETIONS); - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; instance->mark = 1; } + return 0; } /**************************************************************************** @@ -2173,9 +2176,8 @@ vchiq_dump_platform_instances(void *dump_context) * ***************************************************************************/ -void -vchiq_dump_platform_service_state(void *dump_context, - struct vchiq_service *service) +int vchiq_dump_platform_service_state(void *dump_context, + struct vchiq_service *service) { struct user_service *user_service = (struct user_service *)service->base.userdata; @@ -2196,7 +2198,7 @@ vchiq_dump_platform_service_state(void *dump_context, " (dequeue pending)"); } - vchiq_dump(dump_context, buf, len + 1); + return vchiq_dump(dump_context, buf, len + 1); } /**************************************************************************** @@ -2210,13 +2212,16 @@ vchiq_read(struct file *file, char __user *buf, size_t count, loff_t *ppos) { struct dump_context context; + int err; context.buf = buf; context.actual = 0; context.space = count; context.offset = *ppos; - vchiq_dump_state(&context, &g_state); + err = vchiq_dump_state(&context, &g_state); + if (err) + return err; *ppos += context.actual; diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c index b9d94f6b9bef..76351078affb 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c @@ -3298,7 +3298,7 @@ vchiq_set_service_option(unsigned int handle, return status; } -static void +static int vchiq_dump_shared_state(void *dump_context, struct vchiq_state *state, struct vchiq_shared_state *shared, const char *label) { @@ -3318,16 +3318,21 @@ vchiq_dump_shared_state(void *dump_context, struct vchiq_state *state, int i; char buf[80]; int len; + int err; len = scnprintf(buf, sizeof(buf), " %s: slots %d-%d tx_pos=%x recycle=%x", label, shared->slot_first, shared->slot_last, shared->tx_pos, shared->slot_queue_recycle); - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; len = scnprintf(buf, sizeof(buf), " Slots claimed:"); - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; for (i = shared->slot_first; i <= shared->slot_last; i++) { struct vchiq_slot_info slot_info = @@ -3336,27 +3341,34 @@ vchiq_dump_shared_state(void *dump_context, struct vchiq_state *state, len = scnprintf(buf, sizeof(buf), " %d: %d/%d", i, slot_info.use_count, slot_info.release_count); - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; } } for (i = 1; i < shared->debug[DEBUG_ENTRIES]; i++) { len = scnprintf(buf, sizeof(buf), " DEBUG: %s = %d(%x)", debug_names[i], shared->debug[i], shared->debug[i]); - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; } + return 0; } -void -vchiq_dump_state(void *dump_context, struct vchiq_state *state) +int vchiq_dump_state(void *dump_context, struct vchiq_state *state) { char buf[80]; int len; int i; + int err; len = scnprintf(buf, sizeof(buf), "State %d: %s", state->id, conn_state_names[state->conn_state]); - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; len = scnprintf(buf, sizeof(buf), " tx_pos=%x(@%pK), rx_pos=%x(@%pK)", @@ -3364,12 +3376,16 @@ vchiq_dump_state(void *dump_context, struct vchiq_state *state) state->tx_data + (state->local_tx_pos & VCHIQ_SLOT_MASK), state->rx_pos, state->rx_data + (state->rx_pos & VCHIQ_SLOT_MASK)); - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; len = scnprintf(buf, sizeof(buf), " Version: %d (min %d)", VCHIQ_VERSION, VCHIQ_VERSION_MIN); - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; if (VCHIQ_ENABLE_STATS) { len = scnprintf(buf, sizeof(buf), @@ -3377,7 +3393,9 @@ vchiq_dump_state(void *dump_context, struct vchiq_state *state) "error_count=%d", state->stats.ctrl_tx_count, state->stats.ctrl_rx_count, state->stats.error_count); - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; } len = scnprintf(buf, sizeof(buf), @@ -3388,30 +3406,49 @@ vchiq_dump_state(void *dump_context, struct vchiq_state *state) state->data_quota - state->data_use_count, state->local->slot_queue_recycle - state->slot_queue_available, state->stats.slot_stalls, state->stats.data_stalls); - vchiq_dump(dump_context, buf, len + 1); - - vchiq_dump_platform_state(dump_context); - - vchiq_dump_shared_state(dump_context, state, state->local, "Local"); - vchiq_dump_shared_state(dump_context, state, state->remote, "Remote"); - - vchiq_dump_platform_instances(dump_context); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; + + err = vchiq_dump_platform_state(dump_context); + if (err) + return err; + + err = vchiq_dump_shared_state(dump_context, + state, + state->local, + "Local"); + if (err) + return err; + err = vchiq_dump_shared_state(dump_context, + state, + state->remote, + "Remote"); + if (err) + return err; + + err = vchiq_dump_platform_instances(dump_context); + if (err) + return err; for (i = 0; i < state->unused_service; i++) { struct vchiq_service *service = find_service_by_port(state, i); if (service) { - vchiq_dump_service_state(dump_context, service); + err = vchiq_dump_service_state(dump_context, service); unlock_service(service); + if (err) + return err; } } + return 0; } -void -vchiq_dump_service_state(void *dump_context, struct vchiq_service *service) +int vchiq_dump_service_state(void *dump_context, struct vchiq_service *service) { char buf[80]; int len; + int err; len = scnprintf(buf, sizeof(buf), "Service %u: %s (ref %u)", service->localport, srvstate_names[service->srvstate], @@ -3444,7 +3481,9 @@ vchiq_dump_service_state(void *dump_context, struct vchiq_service *service) service_quota->slot_use_count, service_quota->slot_quota); - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; tx_pending = service->bulk_tx.local_insert - service->bulk_tx.remote_insert; @@ -3463,7 +3502,9 @@ vchiq_dump_service_state(void *dump_context, struct vchiq_service *service) BULK_INDEX(service->bulk_rx.remove)].size : 0); if (VCHIQ_ENABLE_STATS) { - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; len = scnprintf(buf, sizeof(buf), " Ctrl: tx_count=%d, tx_bytes=%llu, " @@ -3472,7 +3513,9 @@ vchiq_dump_service_state(void *dump_context, struct vchiq_service *service) service->stats.ctrl_tx_bytes, service->stats.ctrl_rx_count, service->stats.ctrl_rx_bytes); - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; len = scnprintf(buf, sizeof(buf), " Bulk: tx_count=%d, tx_bytes=%llu, " @@ -3481,7 +3524,9 @@ vchiq_dump_service_state(void *dump_context, struct vchiq_service *service) service->stats.bulk_tx_bytes, service->stats.bulk_rx_count, service->stats.bulk_rx_bytes); - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; len = scnprintf(buf, sizeof(buf), " %d quota stalls, %d slot stalls, " @@ -3494,10 +3539,13 @@ vchiq_dump_service_state(void *dump_context, struct vchiq_service *service) } } - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; if (service->srvstate != VCHIQ_SRVSTATE_FREE) - vchiq_dump_platform_service_state(dump_context, service); + err = vchiq_dump_platform_service_state(dump_context, service); + return err; } void diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h index 419bcdd165b4..c31f953a9986 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h @@ -526,10 +526,10 @@ vchiq_bulk_transfer(unsigned int handle, void *offset, int size, void *userdata, enum vchiq_bulk_mode mode, enum vchiq_bulk_dir dir); -extern void +extern int vchiq_dump_state(void *dump_context, struct vchiq_state *state); -extern void +extern int vchiq_dump_service_state(void *dump_context, struct vchiq_service *service); extern void @@ -602,16 +602,16 @@ vchiq_platform_resume(struct vchiq_state *state); extern void vchiq_platform_resumed(struct vchiq_state *state); -extern void +extern int vchiq_dump(void *dump_context, const char *str, int len); -extern void +extern int vchiq_dump_platform_state(void *dump_context); -extern void +extern int vchiq_dump_platform_instances(void *dump_context); -extern void +extern int vchiq_dump_platform_service_state(void *dump_context, struct vchiq_service *service); -- 2.20.1 _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] staging: vchiq: Have vchiq_dump_* functions return an error code 2019-11-20 15:36 ` [PATCH 3/3] staging: vchiq: Have vchiq_dump_* functions return an error code Marcelo Diop-Gonzalez @ 2019-11-20 18:50 ` Dan Carpenter 2019-11-21 21:25 ` Marcelo Diop-Gonzalez 0 siblings, 1 reply; 16+ messages in thread From: Dan Carpenter @ 2019-11-20 18:50 UTC (permalink / raw) To: Marcelo Diop-Gonzalez; +Cc: devel, gregkh, linux-rpi-kernel, wahrenst, eric Thanks for the patch. Looks good. Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com> On Wed, Nov 20, 2019 at 10:36:48AM -0500, Marcelo Diop-Gonzalez wrote: > copy_bytes = min_t(int, len, context->space - context->actual); > if (copy_bytes == 0) > - return; > + return 0; > if (copy_to_user(context->buf + context->actual, str, > copy_bytes)) > - context->actual = -EFAULT; ^^^^^^^^^^^^^^^^^^^^^^^^^ > + return -EFAULT; > context->actual += copy_bytes; ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ In the original code we saved the error code to context->actual, then we added some bytes > len -= copy_bytes; > > @@ -2109,8 +2108,9 @@ vchiq_dump(void *dump_context, const char *str, int len) > > if (copy_to_user(context->buf + context->actual - 1, ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ and then we tried to use the result to corrupt the user space program. So this patch is a nice bug fix. > &cr, 1)) > - context->actual = -EFAULT; > + return -EFAULT; > } > + return 0; > } regards, dan carpenter _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] staging: vchiq: Have vchiq_dump_* functions return an error code 2019-11-20 18:50 ` Dan Carpenter @ 2019-11-21 21:25 ` Marcelo Diop-Gonzalez 0 siblings, 0 replies; 16+ messages in thread From: Marcelo Diop-Gonzalez @ 2019-11-21 21:25 UTC (permalink / raw) To: Dan Carpenter; +Cc: devel, Greg KH, linux-rpi-kernel, Stefan Wahren, eric On Wed, Nov 20, 2019 at 1:51 PM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > Thanks for the patch. Looks good. > > Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com> Appreciate the review! -Marcelo > > On Wed, Nov 20, 2019 at 10:36:48AM -0500, Marcelo Diop-Gonzalez wrote: > > copy_bytes = min_t(int, len, context->space - context->actual); > > if (copy_bytes == 0) > > - return; > > + return 0; > > if (copy_to_user(context->buf + context->actual, str, > > copy_bytes)) > > - context->actual = -EFAULT; > ^^^^^^^^^^^^^^^^^^^^^^^^^ > > > + return -EFAULT; > > context->actual += copy_bytes; > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > In the original code we saved the error code to context->actual, then > we added some bytes > > > len -= copy_bytes; > > > > @@ -2109,8 +2108,9 @@ vchiq_dump(void *dump_context, const char *str, int len) > > > > if (copy_to_user(context->buf + context->actual - 1, > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > and then we tried to use the result to corrupt the user space program. > So this patch is a nice bug fix. > > > &cr, 1)) > > - context->actual = -EFAULT; > > + return -EFAULT; > > } > > + return 0; > > } > > regards, > dan carpenter _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 0/4] staging: vchiq: Fix vchiq_read return value in case of error 2019-11-20 15:36 [PATCH 0/3] staging: vchiq: Fix vchiq_read return value in case of error Marcelo Diop-Gonzalez ` (2 preceding siblings ...) 2019-11-20 15:36 ` [PATCH 3/3] staging: vchiq: Have vchiq_dump_* functions return an error code Marcelo Diop-Gonzalez @ 2019-11-20 20:20 ` Marcelo Diop-Gonzalez 2019-11-20 20:20 ` [PATCH v2 1/4] staging: vchiq: Fix block comment format in vchiq_dump() Marcelo Diop-Gonzalez ` (4 more replies) 2019-11-21 10:42 ` [PATCH 0/3] " Stefan Wahren 4 siblings, 5 replies; 16+ messages in thread From: Marcelo Diop-Gonzalez @ 2019-11-20 20:20 UTC (permalink / raw) To: gregkh, eric, wahrenst; +Cc: devel, linux-rpi-kernel This is a proposed fix of an issue regarding the handling of positive return values from copy_to_user() in vchiq_read(). It looks like the struct dump_context passed to the vchiq_dump() function keeps track of the number of bytes written in the context->actual field. When copy_to_user() returns a positive value, this is set to -EFAULT. The problem is that this is never returned to the user, and instead the function continues, adding the number of bytes that should have been copied to context->actual. Running the following program: #include <stdio.h> #include <stdlib.h> #include <fcntl.h> #include <unistd.h> #include <errno.h> int main(int argc, char **argv) { int fd = open("/dev/vchiq", 0); if (fd < 0) exit(-1); sbrk(getpagesize()); char *bad_buf = sbrk(0)-100; int x = read(fd, bad_buf, 2000); printf("%d %d\n", x, errno); puts(bad_buf); } gives this output: -1 1 State 0: CONNECTED tx_po Remote: slots 2-32 tx_pos=578 recycle=1f Slots claimed: Note the mangled output and incorrect errno value. Messing with the constants in that toy program changes the results. Sometimes read() returns with no error, sometimes it returns with a wrong error code, sometimes with the right one. But it seems that it only ever returns an error at all accidentally, due to the fact that the comparison between context->actual and context->space in vchiq_dump() is unsigned, so that that function won't do anything else if it ever sets context->actual to a negative value. After this patchset, the above program prints this: -1 14 State 0: CONNECTED tx_pos=b4a218(@165de6b4), rx_pos=ae0668(@f02b54f4) Version: 8 (min 3) Stats Help with testing would be appreciated. So far I've basically just diffed the output of 'cat /dev/vchiq', run the program above with a few different values, and run vchiq_test a few times. These were applied to the staging-next branch of the tree at git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git v2: separate first patch into different logical changes. Marcelo Diop-Gonzalez (4): staging: vchiq: Fix block comment format in vchiq_dump() staging: vchiq_dump: Replace min with min_t staging: vchiq: Refactor indentation in vchiq_dump_* functions staging: vchiq: Have vchiq_dump_* functions return an error code .../interface/vchiq_arm/vchiq_2835_arm.c | 5 +- .../interface/vchiq_arm/vchiq_arm.c | 129 ++++++++++-------- .../interface/vchiq_arm/vchiq_core.c | 104 ++++++++++---- .../interface/vchiq_arm/vchiq_core.h | 12 +- 4 files changed, 153 insertions(+), 97 deletions(-) -- 2.24.0.432.g9d3f5f5b63-goog _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 1/4] staging: vchiq: Fix block comment format in vchiq_dump() 2019-11-20 20:20 ` [PATCH v2 0/4] staging: vchiq: Fix vchiq_read return value in case of error Marcelo Diop-Gonzalez @ 2019-11-20 20:20 ` Marcelo Diop-Gonzalez 2019-11-20 20:21 ` [PATCH v2 2/4] staging: vchiq_dump: Replace min with min_t Marcelo Diop-Gonzalez ` (3 subsequent siblings) 4 siblings, 0 replies; 16+ messages in thread From: Marcelo Diop-Gonzalez @ 2019-11-20 20:20 UTC (permalink / raw) To: gregkh, eric, wahrenst; +Cc: devel, linux-rpi-kernel This fixes a checkpatch warning. Signed-off-by: Marcelo Diop-Gonzalez <marcgonzalez@google.com> --- .../staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index 319e7aa0e0e0..6328abcaeeeb 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -2098,9 +2098,11 @@ vchiq_dump(void *dump_context, const char *str, int len) context->actual += copy_bytes; len -= copy_bytes; - /* If tne terminating NUL is included in the length, then it - ** marks the end of a line and should be replaced with a - ** carriage return. */ + /* + * If the terminating NUL is included in the length, then it + * marks the end of a line and should be replaced with a + * carriage return. + */ if ((len == 0) && (str[copy_bytes - 1] == '\0')) { char cr = '\n'; -- 2.24.0.432.g9d3f5f5b63-goog _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 2/4] staging: vchiq_dump: Replace min with min_t 2019-11-20 20:20 ` [PATCH v2 0/4] staging: vchiq: Fix vchiq_read return value in case of error Marcelo Diop-Gonzalez 2019-11-20 20:20 ` [PATCH v2 1/4] staging: vchiq: Fix block comment format in vchiq_dump() Marcelo Diop-Gonzalez @ 2019-11-20 20:21 ` Marcelo Diop-Gonzalez 2019-11-20 20:21 ` [PATCH v2 3/4] staging: vchiq: Refactor indentation in vchiq_dump_* functions Marcelo Diop-Gonzalez ` (2 subsequent siblings) 4 siblings, 0 replies; 16+ messages in thread From: Marcelo Diop-Gonzalez @ 2019-11-20 20:21 UTC (permalink / raw) To: gregkh, eric, wahrenst; +Cc: devel, linux-rpi-kernel Replacing this fixes checkpatch warnings. Signed-off-by: Marcelo Diop-Gonzalez <marcgonzalez@google.com> --- drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index 6328abcaeeeb..942b4768c88e 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -2081,7 +2081,7 @@ vchiq_dump(void *dump_context, const char *str, int len) int copy_bytes; if (context->offset > 0) { - int skip_bytes = min(len, (int)context->offset); + int skip_bytes = min_t(int, len, context->offset); str += skip_bytes; len -= skip_bytes; @@ -2089,7 +2089,7 @@ vchiq_dump(void *dump_context, const char *str, int len) if (context->offset > 0) return; } - copy_bytes = min(len, (int)(context->space - context->actual)); + copy_bytes = min_t(int, len, context->space - context->actual); if (copy_bytes == 0) return; if (copy_to_user(context->buf + context->actual, str, -- 2.24.0.432.g9d3f5f5b63-goog _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 3/4] staging: vchiq: Refactor indentation in vchiq_dump_* functions 2019-11-20 20:20 ` [PATCH v2 0/4] staging: vchiq: Fix vchiq_read return value in case of error Marcelo Diop-Gonzalez 2019-11-20 20:20 ` [PATCH v2 1/4] staging: vchiq: Fix block comment format in vchiq_dump() Marcelo Diop-Gonzalez 2019-11-20 20:21 ` [PATCH v2 2/4] staging: vchiq_dump: Replace min with min_t Marcelo Diop-Gonzalez @ 2019-11-20 20:21 ` Marcelo Diop-Gonzalez 2019-11-20 20:21 ` [PATCH v2 4/4] staging: vchiq: Have vchiq_dump_* functions return an error code Marcelo Diop-Gonzalez 2019-11-21 19:38 ` [PATCH v2 0/4] staging: vchiq: Fix vchiq_read return value in case of error Nicolas Saenz Julienne 4 siblings, 0 replies; 16+ messages in thread From: Marcelo Diop-Gonzalez @ 2019-11-20 20:21 UTC (permalink / raw) To: gregkh, eric, wahrenst; +Cc: devel, linux-rpi-kernel Doing this helps with readability, and makes the logic easier to follow. Signed-off-by: Marcelo Diop-Gonzalez <marcgonzalez@google.com> --- .../interface/vchiq_arm/vchiq_arm.c | 104 +++++++++--------- 1 file changed, 53 insertions(+), 51 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index 942b4768c88e..8f9cfa083264 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -2076,40 +2076,40 @@ void vchiq_dump(void *dump_context, const char *str, int len) { struct dump_context *context = (struct dump_context *)dump_context; + int copy_bytes; - if (context->actual < context->space) { - int copy_bytes; + if (context->actual >= context->space) + return; - if (context->offset > 0) { - int skip_bytes = min_t(int, len, context->offset); + if (context->offset > 0) { + int skip_bytes = min_t(int, len, context->offset); - str += skip_bytes; - len -= skip_bytes; - context->offset -= skip_bytes; - if (context->offset > 0) - return; - } - copy_bytes = min_t(int, len, context->space - context->actual); - if (copy_bytes == 0) + str += skip_bytes; + len -= skip_bytes; + context->offset -= skip_bytes; + if (context->offset > 0) return; - if (copy_to_user(context->buf + context->actual, str, - copy_bytes)) + } + copy_bytes = min_t(int, len, context->space - context->actual); + if (copy_bytes == 0) + return; + if (copy_to_user(context->buf + context->actual, str, + copy_bytes)) + context->actual = -EFAULT; + context->actual += copy_bytes; + len -= copy_bytes; + + /* + * If the terminating NUL is included in the length, then it + * marks the end of a line and should be replaced with a + * carriage return. + */ + if ((len == 0) && (str[copy_bytes - 1] == '\0')) { + char cr = '\n'; + + if (copy_to_user(context->buf + context->actual - 1, + &cr, 1)) context->actual = -EFAULT; - context->actual += copy_bytes; - len -= copy_bytes; - - /* - * If the terminating NUL is included in the length, then it - * marks the end of a line and should be replaced with a - * carriage return. - */ - if ((len == 0) && (str[copy_bytes - 1] == '\0')) { - char cr = '\n'; - - if (copy_to_user(context->buf + context->actual - 1, - &cr, 1)) - context->actual = -EFAULT; - } } } @@ -2134,34 +2134,36 @@ vchiq_dump_platform_instances(void *dump_context) struct vchiq_service *service = state->services[i]; struct vchiq_instance *instance; - if (service && (service->base.callback == service_callback)) { - instance = service->instance; - if (instance) - instance->mark = 0; - } + if (!service || service->base.callback != service_callback) + continue; + + instance = service->instance; + if (instance) + instance->mark = 0; } for (i = 0; i < state->unused_service; i++) { struct vchiq_service *service = state->services[i]; struct vchiq_instance *instance; - if (service && (service->base.callback == service_callback)) { - instance = service->instance; - if (instance && !instance->mark) { - len = snprintf(buf, sizeof(buf), - "Instance %pK: pid %d,%s completions %d/%d", - instance, instance->pid, - instance->connected ? " connected, " : - "", - instance->completion_insert - - instance->completion_remove, - MAX_COMPLETIONS); - - vchiq_dump(dump_context, buf, len + 1); - - instance->mark = 1; - } - } + if (!service || service->base.callback != service_callback) + continue; + + instance = service->instance; + if (!instance || instance->mark) + continue; + + len = snprintf(buf, sizeof(buf), + "Instance %pK: pid %d,%s completions %d/%d", + instance, instance->pid, + instance->connected ? " connected, " : + "", + instance->completion_insert - + instance->completion_remove, + MAX_COMPLETIONS); + + vchiq_dump(dump_context, buf, len + 1); + instance->mark = 1; } } -- 2.24.0.432.g9d3f5f5b63-goog _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 4/4] staging: vchiq: Have vchiq_dump_* functions return an error code 2019-11-20 20:20 ` [PATCH v2 0/4] staging: vchiq: Fix vchiq_read return value in case of error Marcelo Diop-Gonzalez ` (2 preceding siblings ...) 2019-11-20 20:21 ` [PATCH v2 3/4] staging: vchiq: Refactor indentation in vchiq_dump_* functions Marcelo Diop-Gonzalez @ 2019-11-20 20:21 ` Marcelo Diop-Gonzalez 2019-11-21 19:38 ` [PATCH v2 0/4] staging: vchiq: Fix vchiq_read return value in case of error Nicolas Saenz Julienne 4 siblings, 0 replies; 16+ messages in thread From: Marcelo Diop-Gonzalez @ 2019-11-20 20:21 UTC (permalink / raw) To: gregkh, eric, wahrenst; +Cc: devel, linux-rpi-kernel, Dan Carpenter These functions currently modify the struct dump_context passed to them, and set context->actual to -EFAULT in case of error. The issue is that this is never returned to the user (except accidentally when things align so that that happens). So, have these functions return 0 on success and the appropriate error code otherwise, and return nonzero errors to the user. Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Marcelo Diop-Gonzalez <marcgonzalez@google.com> --- .../interface/vchiq_arm/vchiq_2835_arm.c | 5 +- .../interface/vchiq_arm/vchiq_arm.c | 35 +++--- .../interface/vchiq_arm/vchiq_core.c | 104 +++++++++++++----- .../interface/vchiq_arm/vchiq_core.h | 12 +- 4 files changed, 104 insertions(+), 52 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c index e568e9e6eb95..ca30bfd52919 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c @@ -247,15 +247,14 @@ vchiq_complete_bulk(struct vchiq_bulk *bulk) bulk->actual); } -void -vchiq_dump_platform_state(void *dump_context) +int vchiq_dump_platform_state(void *dump_context) { char buf[80]; int len; len = snprintf(buf, sizeof(buf), " Platform: 2835 (VC master)"); - vchiq_dump(dump_context, buf, len + 1); + return vchiq_dump(dump_context, buf, len + 1); } enum vchiq_status diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index 8f9cfa083264..02148a24818a 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -2072,14 +2072,13 @@ static int vchiq_release(struct inode *inode, struct file *file) * ***************************************************************************/ -void -vchiq_dump(void *dump_context, const char *str, int len) +int vchiq_dump(void *dump_context, const char *str, int len) { struct dump_context *context = (struct dump_context *)dump_context; int copy_bytes; if (context->actual >= context->space) - return; + return 0; if (context->offset > 0) { int skip_bytes = min_t(int, len, context->offset); @@ -2088,14 +2087,14 @@ vchiq_dump(void *dump_context, const char *str, int len) len -= skip_bytes; context->offset -= skip_bytes; if (context->offset > 0) - return; + return 0; } copy_bytes = min_t(int, len, context->space - context->actual); if (copy_bytes == 0) - return; + return 0; if (copy_to_user(context->buf + context->actual, str, copy_bytes)) - context->actual = -EFAULT; + return -EFAULT; context->actual += copy_bytes; len -= copy_bytes; @@ -2109,8 +2108,9 @@ vchiq_dump(void *dump_context, const char *str, int len) if (copy_to_user(context->buf + context->actual - 1, &cr, 1)) - context->actual = -EFAULT; + return -EFAULT; } + return 0; } /**************************************************************************** @@ -2119,8 +2119,7 @@ vchiq_dump(void *dump_context, const char *str, int len) * ***************************************************************************/ -void -vchiq_dump_platform_instances(void *dump_context) +int vchiq_dump_platform_instances(void *dump_context) { struct vchiq_state *state = vchiq_get_state(); char buf[80]; @@ -2145,6 +2144,7 @@ vchiq_dump_platform_instances(void *dump_context) for (i = 0; i < state->unused_service; i++) { struct vchiq_service *service = state->services[i]; struct vchiq_instance *instance; + int err; if (!service || service->base.callback != service_callback) continue; @@ -2162,9 +2162,12 @@ vchiq_dump_platform_instances(void *dump_context) instance->completion_remove, MAX_COMPLETIONS); - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; instance->mark = 1; } + return 0; } /**************************************************************************** @@ -2173,9 +2176,8 @@ vchiq_dump_platform_instances(void *dump_context) * ***************************************************************************/ -void -vchiq_dump_platform_service_state(void *dump_context, - struct vchiq_service *service) +int vchiq_dump_platform_service_state(void *dump_context, + struct vchiq_service *service) { struct user_service *user_service = (struct user_service *)service->base.userdata; @@ -2196,7 +2198,7 @@ vchiq_dump_platform_service_state(void *dump_context, " (dequeue pending)"); } - vchiq_dump(dump_context, buf, len + 1); + return vchiq_dump(dump_context, buf, len + 1); } /**************************************************************************** @@ -2210,13 +2212,16 @@ vchiq_read(struct file *file, char __user *buf, size_t count, loff_t *ppos) { struct dump_context context; + int err; context.buf = buf; context.actual = 0; context.space = count; context.offset = *ppos; - vchiq_dump_state(&context, &g_state); + err = vchiq_dump_state(&context, &g_state); + if (err) + return err; *ppos += context.actual; diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c index b9d94f6b9bef..76351078affb 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c @@ -3298,7 +3298,7 @@ vchiq_set_service_option(unsigned int handle, return status; } -static void +static int vchiq_dump_shared_state(void *dump_context, struct vchiq_state *state, struct vchiq_shared_state *shared, const char *label) { @@ -3318,16 +3318,21 @@ vchiq_dump_shared_state(void *dump_context, struct vchiq_state *state, int i; char buf[80]; int len; + int err; len = scnprintf(buf, sizeof(buf), " %s: slots %d-%d tx_pos=%x recycle=%x", label, shared->slot_first, shared->slot_last, shared->tx_pos, shared->slot_queue_recycle); - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; len = scnprintf(buf, sizeof(buf), " Slots claimed:"); - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; for (i = shared->slot_first; i <= shared->slot_last; i++) { struct vchiq_slot_info slot_info = @@ -3336,27 +3341,34 @@ vchiq_dump_shared_state(void *dump_context, struct vchiq_state *state, len = scnprintf(buf, sizeof(buf), " %d: %d/%d", i, slot_info.use_count, slot_info.release_count); - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; } } for (i = 1; i < shared->debug[DEBUG_ENTRIES]; i++) { len = scnprintf(buf, sizeof(buf), " DEBUG: %s = %d(%x)", debug_names[i], shared->debug[i], shared->debug[i]); - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; } + return 0; } -void -vchiq_dump_state(void *dump_context, struct vchiq_state *state) +int vchiq_dump_state(void *dump_context, struct vchiq_state *state) { char buf[80]; int len; int i; + int err; len = scnprintf(buf, sizeof(buf), "State %d: %s", state->id, conn_state_names[state->conn_state]); - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; len = scnprintf(buf, sizeof(buf), " tx_pos=%x(@%pK), rx_pos=%x(@%pK)", @@ -3364,12 +3376,16 @@ vchiq_dump_state(void *dump_context, struct vchiq_state *state) state->tx_data + (state->local_tx_pos & VCHIQ_SLOT_MASK), state->rx_pos, state->rx_data + (state->rx_pos & VCHIQ_SLOT_MASK)); - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; len = scnprintf(buf, sizeof(buf), " Version: %d (min %d)", VCHIQ_VERSION, VCHIQ_VERSION_MIN); - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; if (VCHIQ_ENABLE_STATS) { len = scnprintf(buf, sizeof(buf), @@ -3377,7 +3393,9 @@ vchiq_dump_state(void *dump_context, struct vchiq_state *state) "error_count=%d", state->stats.ctrl_tx_count, state->stats.ctrl_rx_count, state->stats.error_count); - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; } len = scnprintf(buf, sizeof(buf), @@ -3388,30 +3406,49 @@ vchiq_dump_state(void *dump_context, struct vchiq_state *state) state->data_quota - state->data_use_count, state->local->slot_queue_recycle - state->slot_queue_available, state->stats.slot_stalls, state->stats.data_stalls); - vchiq_dump(dump_context, buf, len + 1); - - vchiq_dump_platform_state(dump_context); - - vchiq_dump_shared_state(dump_context, state, state->local, "Local"); - vchiq_dump_shared_state(dump_context, state, state->remote, "Remote"); - - vchiq_dump_platform_instances(dump_context); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; + + err = vchiq_dump_platform_state(dump_context); + if (err) + return err; + + err = vchiq_dump_shared_state(dump_context, + state, + state->local, + "Local"); + if (err) + return err; + err = vchiq_dump_shared_state(dump_context, + state, + state->remote, + "Remote"); + if (err) + return err; + + err = vchiq_dump_platform_instances(dump_context); + if (err) + return err; for (i = 0; i < state->unused_service; i++) { struct vchiq_service *service = find_service_by_port(state, i); if (service) { - vchiq_dump_service_state(dump_context, service); + err = vchiq_dump_service_state(dump_context, service); unlock_service(service); + if (err) + return err; } } + return 0; } -void -vchiq_dump_service_state(void *dump_context, struct vchiq_service *service) +int vchiq_dump_service_state(void *dump_context, struct vchiq_service *service) { char buf[80]; int len; + int err; len = scnprintf(buf, sizeof(buf), "Service %u: %s (ref %u)", service->localport, srvstate_names[service->srvstate], @@ -3444,7 +3481,9 @@ vchiq_dump_service_state(void *dump_context, struct vchiq_service *service) service_quota->slot_use_count, service_quota->slot_quota); - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; tx_pending = service->bulk_tx.local_insert - service->bulk_tx.remote_insert; @@ -3463,7 +3502,9 @@ vchiq_dump_service_state(void *dump_context, struct vchiq_service *service) BULK_INDEX(service->bulk_rx.remove)].size : 0); if (VCHIQ_ENABLE_STATS) { - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; len = scnprintf(buf, sizeof(buf), " Ctrl: tx_count=%d, tx_bytes=%llu, " @@ -3472,7 +3513,9 @@ vchiq_dump_service_state(void *dump_context, struct vchiq_service *service) service->stats.ctrl_tx_bytes, service->stats.ctrl_rx_count, service->stats.ctrl_rx_bytes); - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; len = scnprintf(buf, sizeof(buf), " Bulk: tx_count=%d, tx_bytes=%llu, " @@ -3481,7 +3524,9 @@ vchiq_dump_service_state(void *dump_context, struct vchiq_service *service) service->stats.bulk_tx_bytes, service->stats.bulk_rx_count, service->stats.bulk_rx_bytes); - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; len = scnprintf(buf, sizeof(buf), " %d quota stalls, %d slot stalls, " @@ -3494,10 +3539,13 @@ vchiq_dump_service_state(void *dump_context, struct vchiq_service *service) } } - vchiq_dump(dump_context, buf, len + 1); + err = vchiq_dump(dump_context, buf, len + 1); + if (err) + return err; if (service->srvstate != VCHIQ_SRVSTATE_FREE) - vchiq_dump_platform_service_state(dump_context, service); + err = vchiq_dump_platform_service_state(dump_context, service); + return err; } void diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h index 419bcdd165b4..c31f953a9986 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h @@ -526,10 +526,10 @@ vchiq_bulk_transfer(unsigned int handle, void *offset, int size, void *userdata, enum vchiq_bulk_mode mode, enum vchiq_bulk_dir dir); -extern void +extern int vchiq_dump_state(void *dump_context, struct vchiq_state *state); -extern void +extern int vchiq_dump_service_state(void *dump_context, struct vchiq_service *service); extern void @@ -602,16 +602,16 @@ vchiq_platform_resume(struct vchiq_state *state); extern void vchiq_platform_resumed(struct vchiq_state *state); -extern void +extern int vchiq_dump(void *dump_context, const char *str, int len); -extern void +extern int vchiq_dump_platform_state(void *dump_context); -extern void +extern int vchiq_dump_platform_instances(void *dump_context); -extern void +extern int vchiq_dump_platform_service_state(void *dump_context, struct vchiq_service *service); -- 2.24.0.432.g9d3f5f5b63-goog _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/4] staging: vchiq: Fix vchiq_read return value in case of error 2019-11-20 20:20 ` [PATCH v2 0/4] staging: vchiq: Fix vchiq_read return value in case of error Marcelo Diop-Gonzalez ` (3 preceding siblings ...) 2019-11-20 20:21 ` [PATCH v2 4/4] staging: vchiq: Have vchiq_dump_* functions return an error code Marcelo Diop-Gonzalez @ 2019-11-21 19:38 ` Nicolas Saenz Julienne 2019-11-21 21:24 ` Marcelo Diop-Gonzalez 4 siblings, 1 reply; 16+ messages in thread From: Nicolas Saenz Julienne @ 2019-11-21 19:38 UTC (permalink / raw) To: Marcelo Diop-Gonzalez, gregkh, eric, wahrenst; +Cc: devel, linux-rpi-kernel [-- Attachment #1.1: Type: text/plain, Size: 2400 bytes --] Hi Marcelo, On Wed, 2019-11-20 at 15:20 -0500, Marcelo Diop-Gonzalez wrote: > This is a proposed fix of an issue regarding the handling of positive > return values from copy_to_user() in vchiq_read(). It looks like the > struct dump_context passed to the vchiq_dump() function keeps track > of the number of bytes written in the context->actual field. When > copy_to_user() returns a positive value, this is set to -EFAULT. The > problem is that this is never returned to the user, and instead the > function continues, adding the number of bytes that should have > been copied to context->actual. > > Running the following program: > > #include <stdio.h> > #include <stdlib.h> > #include <fcntl.h> > #include <unistd.h> > #include <errno.h> > > int main(int argc, char **argv) { > int fd = open("/dev/vchiq", 0); > if (fd < 0) > exit(-1); > sbrk(getpagesize()); > char *bad_buf = sbrk(0)-100; > int x = read(fd, bad_buf, 2000); > printf("%d %d\n", x, errno); > puts(bad_buf); > } > > gives this output: > > -1 1 > State 0: CONNECTED > tx_po > > > Remote: slots 2-32 tx_pos=578 recycle=1f > Slots claimed: > > Note the mangled output and incorrect errno value. Messing with the > constants in that toy program changes the results. Sometimes read() > returns with no error, sometimes it returns with a wrong error code, > sometimes with the right one. But it seems that it only ever returns an > error at all accidentally, due to the fact that the comparison between > context->actual and context->space in vchiq_dump() is unsigned, so that > that function won't do anything else if it ever sets context->actual > to a negative value. > > After this patchset, the above program prints this: > > -1 14 > State 0: CONNECTED > tx_pos=b4a218(@165de6b4), rx_pos=ae0668(@f02b54f4) > Version: 8 (min 3) > Stats > > Help with testing would be appreciated. So far I've basically just > diffed the output of 'cat /dev/vchiq', run the program above with > a few different values, and run vchiq_test a few times. > > These were applied to the staging-next branch of the tree > at git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git For the whole series: Reviewed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> Thanks, Nicolas [-- Attachment #1.2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 169 bytes --] _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/4] staging: vchiq: Fix vchiq_read return value in case of error 2019-11-21 19:38 ` [PATCH v2 0/4] staging: vchiq: Fix vchiq_read return value in case of error Nicolas Saenz Julienne @ 2019-11-21 21:24 ` Marcelo Diop-Gonzalez 0 siblings, 0 replies; 16+ messages in thread From: Marcelo Diop-Gonzalez @ 2019-11-21 21:24 UTC (permalink / raw) To: Nicolas Saenz Julienne Cc: devel, Greg KH, linux-rpi-kernel, Stefan Wahren, eric On Thu, Nov 21, 2019 at 2:39 PM Nicolas Saenz Julienne <nsaenzjulienne@suse.de> wrote: > > Hi Marcelo, > > On Wed, 2019-11-20 at 15:20 -0500, Marcelo Diop-Gonzalez wrote: > > This is a proposed fix of an issue regarding the handling of positive > > return values from copy_to_user() in vchiq_read(). It looks like the > > struct dump_context passed to the vchiq_dump() function keeps track > > of the number of bytes written in the context->actual field. When > > copy_to_user() returns a positive value, this is set to -EFAULT. The > > problem is that this is never returned to the user, and instead the > > function continues, adding the number of bytes that should have > > been copied to context->actual. > > > > Running the following program: > > > > #include <stdio.h> > > #include <stdlib.h> > > #include <fcntl.h> > > #include <unistd.h> > > #include <errno.h> > > > > int main(int argc, char **argv) { > > int fd = open("/dev/vchiq", 0); > > if (fd < 0) > > exit(-1); > > sbrk(getpagesize()); > > char *bad_buf = sbrk(0)-100; > > int x = read(fd, bad_buf, 2000); > > printf("%d %d\n", x, errno); > > puts(bad_buf); > > } > > > > gives this output: > > > > -1 1 > > State 0: CONNECTED > > tx_po > > > > > > Remote: slots 2-32 tx_pos=578 recycle=1f > > Slots claimed: > > > > Note the mangled output and incorrect errno value. Messing with the > > constants in that toy program changes the results. Sometimes read() > > returns with no error, sometimes it returns with a wrong error code, > > sometimes with the right one. But it seems that it only ever returns an > > error at all accidentally, due to the fact that the comparison between > > context->actual and context->space in vchiq_dump() is unsigned, so that > > that function won't do anything else if it ever sets context->actual > > to a negative value. > > > > After this patchset, the above program prints this: > > > > -1 14 > > State 0: CONNECTED > > tx_pos=b4a218(@165de6b4), rx_pos=ae0668(@f02b54f4) > > Version: 8 (min 3) > > Stats > > > > Help with testing would be appreciated. So far I've basically just > > diffed the output of 'cat /dev/vchiq', run the program above with > > a few different values, and run vchiq_test a few times. > > > > These were applied to the staging-next branch of the tree > > at git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git > > For the whole series: > > Reviewed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> Thanks for reviewing! -Marcelo > > Thanks, > Nicolas > _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] staging: vchiq: Fix vchiq_read return value in case of error 2019-11-20 15:36 [PATCH 0/3] staging: vchiq: Fix vchiq_read return value in case of error Marcelo Diop-Gonzalez ` (3 preceding siblings ...) 2019-11-20 20:20 ` [PATCH v2 0/4] staging: vchiq: Fix vchiq_read return value in case of error Marcelo Diop-Gonzalez @ 2019-11-21 10:42 ` Stefan Wahren 2019-11-21 19:41 ` Nicolas Saenz Julienne 4 siblings, 1 reply; 16+ messages in thread From: Stefan Wahren @ 2019-11-21 10:42 UTC (permalink / raw) To: Marcelo Diop-Gonzalez, gregkh, eric, wahrenst Cc: devel, Phil Elwell, linux-rpi-kernel Hi Marcelo, [add Nicolas and Phil] Am 20.11.19 um 16:36 schrieb Marcelo Diop-Gonzalez: > This is a proposed fix of an issue regarding the handling of positive > return values from copy_to_user() in vchiq_read(). It looks like the > struct dump_context passed to the vchiq_dump() function keeps track > of the number of bytes written in the context->actual field. When > copy_to_user() returns a positive value, this is set to -EFAULT. The > problem is that this is never returned to the user, and instead the function > continues, adding the number of bytes that should have been copied > to context->actual. > > Running the following program: > > #include <stdio.h> > #include <stdlib.h> > #include <fcntl.h> > #include <unistd.h> > #include <errno.h> > > int main(int argc, char **argv) { > int fd = open("/dev/vchiq", 0); > if (fd < 0) > exit(-1); > sbrk(getpagesize()); > char *bad_buf = sbrk(0)-100; > int x = read(fd, bad_buf, 2000); > printf("%d %d\n", x, errno); > puts(bad_buf); > } > > gives this output: > > -1 1 > State 0: CONNECTED > tx_po > > > Remote: slots 2-32 tx_pos=578 recycle=1f > Slots claimed: > > Note the mangled output and incorrect errno value. Messing with the > constants in that toy program changes the results. Sometimes read() > returns with no error, sometimes it returns with a wrong error code, > sometimes with the right one. But it seems that it only ever returns an > error at all accidentally, due to the fact that the comparison between > context->actual and context->space in vchiq_dump() is unsigned, so that > that function won't do anything else if it ever sets context->actual > to a negative value. > > After this patchset, the above program prints this: > > -1 14 > State 0: CONNECTED > tx_pos=b4a218(@165de6b4), rx_pos=ae0668(@f02b54f4) > Version: 8 (min 3) > Stats > > Help with testing would be appreciated. So far I've basically just > diffed the output of 'cat /dev/vchiq', run the program above with > a few different values, and run vchiq_test a few times. i consider this as sufficient, but i'm not the VCHIQ expert. Thanks Stefan > > These were applied to the staging-next branch of the tree > at git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git > > Marcelo Diop-Gonzalez (3): > staging: vchiq_dump: Replace min with min_t > staging: vchiq: Refactor indentation in vchiq_dump_* functions > staging: vchiq: Have vchiq_dump_* functions return an error code > > .../interface/vchiq_arm/vchiq_2835_arm.c | 5 +- > .../interface/vchiq_arm/vchiq_arm.c | 129 ++++++++++-------- > .../interface/vchiq_arm/vchiq_core.c | 104 ++++++++++---- > .../interface/vchiq_arm/vchiq_core.h | 12 +- > 4 files changed, 153 insertions(+), 97 deletions(-) > _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] staging: vchiq: Fix vchiq_read return value in case of error 2019-11-21 10:42 ` [PATCH 0/3] " Stefan Wahren @ 2019-11-21 19:41 ` Nicolas Saenz Julienne 0 siblings, 0 replies; 16+ messages in thread From: Nicolas Saenz Julienne @ 2019-11-21 19:41 UTC (permalink / raw) To: Stefan Wahren, Marcelo Diop-Gonzalez, gregkh, eric Cc: devel, Phil Elwell, linux-rpi-kernel [-- Attachment #1.1: Type: text/plain, Size: 452 bytes --] On Thu, 2019-11-21 at 11:42 +0100, Stefan Wahren wrote: [...] > > Help with testing would be appreciated. So far I've basically just > > diffed the output of 'cat /dev/vchiq', run the program above with > > a few different values, and run vchiq_test a few times. > > i consider this as sufficient, but i'm not the VCHIQ expert. Agree, the patch only affects the vchiq state dump code, I think it's OK with this test. Regards, Nicolas [-- Attachment #1.2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 169 bytes --] _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-11-21 21:25 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-11-20 15:36 [PATCH 0/3] staging: vchiq: Fix vchiq_read return value in case of error Marcelo Diop-Gonzalez 2019-11-20 15:36 ` [PATCH 1/3] staging: vchiq_dump: Replace min with min_t Marcelo Diop-Gonzalez 2019-11-20 15:46 ` Greg KH 2019-11-20 15:36 ` [PATCH 2/3] staging: vchiq: Refactor indentation in vchiq_dump_* functions Marcelo Diop-Gonzalez 2019-11-20 15:36 ` [PATCH 3/3] staging: vchiq: Have vchiq_dump_* functions return an error code Marcelo Diop-Gonzalez 2019-11-20 18:50 ` Dan Carpenter 2019-11-21 21:25 ` Marcelo Diop-Gonzalez 2019-11-20 20:20 ` [PATCH v2 0/4] staging: vchiq: Fix vchiq_read return value in case of error Marcelo Diop-Gonzalez 2019-11-20 20:20 ` [PATCH v2 1/4] staging: vchiq: Fix block comment format in vchiq_dump() Marcelo Diop-Gonzalez 2019-11-20 20:21 ` [PATCH v2 2/4] staging: vchiq_dump: Replace min with min_t Marcelo Diop-Gonzalez 2019-11-20 20:21 ` [PATCH v2 3/4] staging: vchiq: Refactor indentation in vchiq_dump_* functions Marcelo Diop-Gonzalez 2019-11-20 20:21 ` [PATCH v2 4/4] staging: vchiq: Have vchiq_dump_* functions return an error code Marcelo Diop-Gonzalez 2019-11-21 19:38 ` [PATCH v2 0/4] staging: vchiq: Fix vchiq_read return value in case of error Nicolas Saenz Julienne 2019-11-21 21:24 ` Marcelo Diop-Gonzalez 2019-11-21 10:42 ` [PATCH 0/3] " Stefan Wahren 2019-11-21 19:41 ` Nicolas Saenz Julienne
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).