[BlueZ] mesh: Fix application key binding lookup

Commit Message

Michał Lowas-Rzechonek Aug. 11, 2020, 1:39 p.m. UTC

Because l_queue_find can't distinguish between entry->data equal to zero
and missing entry, has_binding() fails when we bind app key with index
0, via L_UINT_TO_PTR.

Bug has been introduced in commit 1a2a6debd
---
 mesh/model.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

[e-mail] Aug. 11, 2020, 1:50 p.m. UTC | #1

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
While we are preparing for reviewing the patches, we found the following
issue/warning.

Test Result:
checkpatch Failed

Outputs:
ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 1a2a6debd00a ("mesh: Clean up handling of config model binding messages")'
#11: 
Bug has been introduced in commit 1a2a6debd

WARNING:BLOCK_COMMENT_STYLE: Block comments use a trailing */ on a separate line
#23: FILE: mesh/model.c:115:
+	 * and entry with data equal to NULL */

- total: 1 errors, 1 warnings, 16 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.



---
Regards,
Linux Bluetooth

Brian Gix Aug. 11, 2020, 2:45 p.m. UTC | #2

Applied, Thanks.

On Tue, 2020-08-11 at 15:39 +0200, Michał Lowas-Rzechonek wrote:
> Because l_queue_find can't distinguish between entry->data equal to zero
> and missing entry, has_binding() fails when we bind app key with index
> 0, via L_UINT_TO_PTR.
> 
> Bug has been introduced in commit 1a2a6debd
> ---
>  mesh/model.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/mesh/model.c b/mesh/model.c
> index 4eaad31be..7d7f1551a 100644
> --- a/mesh/model.c
> +++ b/mesh/model.c
> @@ -140,7 +140,15 @@ static bool simple_match(const void *a, const void *b)
>  
>  static bool has_binding(struct l_queue *bindings, uint16_t idx)
>  {
> -	return l_queue_find(bindings, simple_match, L_UINT_TO_PTR(idx)) != NULL;
> +	/* don't use l_queue_find, it can't distinguish between missing entry
> +	 * and entry with data equal to NULL */
> +	const struct l_queue_entry *entry;
> +
> +	for (entry = l_queue_get_entries(bindings); entry; entry = entry->next)
> +		if (L_PTR_TO_INT(entry->data) == idx)
> +			return true;
> +
> +	return false;
>  }
>  
>  static bool find_virt_by_label(const void *a, const void *b)

Patch

diff --git a/mesh/model.c b/mesh/model.c
index 4eaad31be..7d7f1551a 100644
--- a/mesh/model.c
+++ b/mesh/model.c
@@ -140,7 +140,15 @@  static bool simple_match(const void *a, const void *b)
 
 static bool has_binding(struct l_queue *bindings, uint16_t idx)
 {
-	return l_queue_find(bindings, simple_match, L_UINT_TO_PTR(idx)) != NULL;
+	/* don't use l_queue_find, it can't distinguish between missing entry
+	 * and entry with data equal to NULL */
+	const struct l_queue_entry *entry;
+
+	for (entry = l_queue_get_entries(bindings); entry; entry = entry->next)
+		if (L_PTR_TO_INT(entry->data) == idx)
+			return true;
+
+	return false;
 }
 
 static bool find_virt_by_label(const void *a, const void *b)