2020-03-31 09:25:14

by Michał Lowas-Rzechonek

Subject: [PATCH BlueZ v2] mesh: Fix FPE in overcommit logic.

During overcommit, mesh_config_save is called in asynchronous mode to
avoid blocking Send() calls. This means that update of cfg->write_time
is scheduled via l_idle_oneshot, so if the next Send() gets scheduled
first, the code may see elapsed time of zero.

If this happens, then the overcommit logic was already executed and the
overcommit is pending, so we can just return.
---
Fixed the commit log, sorry.

mesh/mesh-config-json.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/mesh/mesh-config-json.c b/mesh/mesh-config-json.c
index ad2d4d0f8..21f2cfc37 100644
--- a/mesh/mesh-config-json.c
+++ b/mesh/mesh-config-json.c
@@ -2014,6 +2014,12 @@ bool mesh_config_write_seq_number(struct mesh_config *cfg, uint32_t seq,
timersub(&now, &cfg->write_time, &elapsed);
elapsed_ms = elapsed.tv_sec * 1000 + elapsed.tv_usec / 1000;

+ /* If time since last write is zero, this means that
+ * idle_save_config is already pending, so we don't need to do
+ * anything. */
+ if (!elapsed_ms)
+ return true;
+
cached = seq + (seq - cfg->write_seq) *
1000 * MIN_SEQ_CACHE_TIME / elapsed_ms;

--
2.20.1

2020-04-03 17:55:52

by Michał Lowas-Rzechonek

Subject: Re: [PATCH BlueZ v2] mesh: Fix FPE in overcommit logic.

Brian,

On 03/31, Michał Lowas-Rzechonek wrote:
> During overcommit, mesh_config_save is called in asynchronous mode to
> avoid blocking Send() calls. This means that update of cfg->write_time
> is scheduled via l_idle_oneshot, so if the next Send() gets scheduled
> first, the code may see elapsed time of zero.
>
> If this happens, then the overcommit logic was already executed and the
> overcommit is pending, so we can just return.

Ping?

--
Michał Lowas-Rzechonek <[email protected]>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND

2020-06-02 16:11:45

by Gix, Brian

Subject: Re: [PATCH BlueZ v2] mesh: Fix FPE in overcommit logic.

Applied with style guide fixes

On Tue, 2020-03-31 at 11:04 +0200, Michał Lowas-Rzechonek wrote:
> During overcommit, mesh_config_save is called in asynchronous mode to
> avoid blocking Send() calls. This means that update of cfg->write_time
> is scheduled via l_idle_oneshot, so if the next Send() gets scheduled
> first, the code may see elapsed time of zero.
>
> If this happens, then the overcommit logic was already executed and the
> overcommit is pending, so we can just return.
> ---
> Fixed the commit log, sorry.
>
> mesh/mesh-config-json.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/mesh/mesh-config-json.c b/mesh/mesh-config-json.c
> index ad2d4d0f8..21f2cfc37 100644
> --- a/mesh/mesh-config-json.c
> +++ b/mesh/mesh-config-json.c
> @@ -2014,6 +2014,12 @@ bool mesh_config_write_seq_number(struct mesh_config *cfg, uint32_t seq,
> timersub(&now, &cfg->write_time, &elapsed);
> elapsed_ms = elapsed.tv_sec * 1000 + elapsed.tv_usec / 1000;
>
> + /* If time since last write is zero, this means that
> + * idle_save_config is already pending, so we don't need to do
> + * anything. */
> + if (!elapsed_ms)
> + return true;
> +
> cached = seq + (seq - cfg->write_seq) *
> 1000 * MIN_SEQ_CACHE_TIME / elapsed_ms;
>