No change in functioanality, code tightening.
---
mesh/keyring.c | 120 +++++++++++++++++++++----------------------------
1 file changed, 50 insertions(+), 70 deletions(-)
diff --git a/mesh/keyring.c b/mesh/keyring.c
index 0b74ee914..025703574 100644
--- a/mesh/keyring.c
+++ b/mesh/keyring.c
@@ -33,31 +33,45 @@ const char *dev_key_dir = "/dev_keys";
const char *app_key_dir = "/app_keys";
const char *net_key_dir = "/net_keys";
-bool keyring_put_net_key(struct mesh_node *node, uint16_t net_idx,
- struct keyring_net_key *key)
+static int open_key_file(struct mesh_node *node, const char *key_dir,
+ uint16_t idx, bool is_wrt, int flags)
{
const char *node_path;
- char key_file[PATH_MAX];
- bool result = false;
- int fd;
+ char fname[PATH_MAX];
- if (!node || !key)
- return false;
+ if (!node)
+ return -1;
node_path = node_get_storage_dir(node);
- if (strlen(node_path) + strlen(net_key_dir) + 1 + 3 >= PATH_MAX)
- return false;
+ if (strlen(node_path) + strlen(key_dir) + 1 + 3 >= PATH_MAX)
+ return -1;
- snprintf(key_file, PATH_MAX, "%s%s", node_path, net_key_dir);
+ if (is_wrt) {
+ snprintf(fname, PATH_MAX, "%s%s", node_path, key_dir);
+ mkdir(fname, 0755);
+ }
- mkdir(key_file, 0755);
+ snprintf(fname, PATH_MAX, "%s%s/%3.3x", node_path, key_dir, idx);
- snprintf(key_file, PATH_MAX, "%s%s/%3.3x", node_path, net_key_dir,
- net_idx);
- l_debug("Put Net Key %s", key_file);
+ if (is_wrt)
+ return open(fname, flags, S_IRUSR | S_IWUSR);
+ else
+ return open(fname, flags);
+}
+
+bool keyring_put_net_key(struct mesh_node *node, uint16_t net_idx,
+ struct keyring_net_key *key)
+{
+ bool result = false;
+ int fd;
+
+ if (!key)
+ return false;
+
+ fd = open_key_file(node, net_key_dir, net_idx, true,
+ O_WRONLY | O_CREAT | O_TRUNC);
- fd = open(key_file, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR);
if (fd < 0)
return false;
@@ -72,28 +86,14 @@ bool keyring_put_net_key(struct mesh_node *node, uint16_t net_idx,
bool keyring_put_app_key(struct mesh_node *node, uint16_t app_idx,
uint16_t net_idx, struct keyring_app_key *key)
{
- const char *node_path;
- char key_file[PATH_MAX];
bool result = false;
int fd;
- if (!node || !key)
+ if (!key)
return false;
- node_path = node_get_storage_dir(node);
-
- if (strlen(node_path) + strlen(app_key_dir) + 1 + 3 >= PATH_MAX)
- return false;
+ fd = open_key_file(node, app_key_dir, app_idx, false, O_RDWR);
- snprintf(key_file, PATH_MAX, "%s%s", node_path, app_key_dir);
-
- mkdir(key_file, 0755);
-
- snprintf(key_file, PATH_MAX, "%s%s/%3.3x", node_path, app_key_dir,
- app_idx);
- l_debug("Put App Key %s", key_file);
-
- fd = open(key_file, O_RDWR);
if (fd >= 0) {
struct keyring_app_key old_key;
@@ -105,12 +105,12 @@ bool keyring_put_app_key(struct mesh_node *node, uint16_t app_idx,
}
lseek(fd, 0, SEEK_SET);
- } else {
- fd = open(key_file, O_WRONLY | O_CREAT | O_TRUNC,
- S_IRUSR | S_IWUSR);
- if (fd < 0)
- return false;
- }
+ } else
+ fd = open_key_file(node, app_key_dir, app_idx, true,
+ O_WRONLY | O_CREAT | O_TRUNC);
+
+ if (fd < 0)
+ return false;
if (write(fd, key, sizeof(*key)) == sizeof(*key))
result = true;
@@ -212,8 +212,7 @@ bool keyring_put_remote_dev_key(struct mesh_node *node, uint16_t unicast,
dev_key_dir, unicast + i);
l_debug("Put Dev Key %s", key_file);
- fd = open(key_file, O_WRONLY | O_CREAT | O_TRUNC,
- S_IRUSR | S_IWUSR);
+ fd = open(key_file, O_WRONLY | O_CREAT | O_TRUNC, 0600);
if (fd >= 0) {
if (write(fd, dev_key, 16) != 16)
result = false;
@@ -226,24 +225,19 @@ bool keyring_put_remote_dev_key(struct mesh_node *node, uint16_t unicast,
return result;
}
-bool keyring_get_net_key(struct mesh_node *node, uint16_t net_idx,
- struct keyring_net_key *key)
+static bool get_key(struct mesh_node *node, const char *key_dir,
+ uint16_t key_idx, void *key, ssize_t sz)
{
- const char *node_path;
- char key_file[PATH_MAX];
bool result = false;
int fd;
- if (!node || !key)
+ if (!key)
return false;
- node_path = node_get_storage_dir(node);
- snprintf(key_file, PATH_MAX, "%s%s/%3.3x", node_path, net_key_dir,
- net_idx);
+ fd = open_key_file(node, key_dir, key_idx, false, O_RDONLY);
- fd = open(key_file, O_RDONLY);
if (fd >= 0) {
- if (read(fd, key, sizeof(*key)) == sizeof(*key))
+ if (read(fd, key, sz) == sz)
result = true;
close(fd);
@@ -252,30 +246,16 @@ bool keyring_get_net_key(struct mesh_node *node, uint16_t net_idx,
return result;
}
+bool keyring_get_net_key(struct mesh_node *node, uint16_t net_idx,
+ struct keyring_net_key *key)
+{
+ return get_key(node, net_key_dir, net_idx, key, sizeof(*key));
+}
+
bool keyring_get_app_key(struct mesh_node *node, uint16_t app_idx,
struct keyring_app_key *key)
{
- const char *node_path;
- char key_file[PATH_MAX];
- bool result = false;
- int fd;
-
- if (!node || !key)
- return false;
-
- node_path = node_get_storage_dir(node);
- snprintf(key_file, PATH_MAX, "%s%s/%3.3x", node_path, app_key_dir,
- app_idx);
-
- fd = open(key_file, O_RDONLY);
- if (fd >= 0) {
- if (read(fd, key, sizeof(*key)) == sizeof(*key))
- result = true;
-
- close(fd);
- }
-
- return result;
+ return get_key(node, app_key_dir, app_idx, key, sizeof(*key));
}
bool keyring_get_remote_dev_key(struct mesh_node *node, uint16_t unicast,
--
2.26.2
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.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=434249
---Test result---
##############################
Test: CheckPatch - FAIL
Output:
mesh: Combine common functions for NetKeys and AppKeys
WARNING:SYMBOLIC_PERMS: Symbolic permissions 'S_IRUSR | S_IWUSR' are not preferred. Consider using octal permissions '0600'.
#52: FILE: mesh/keyring.c:58:
+ return open(fname, flags, S_IRUSR | S_IWUSR);
- total: 0 errors, 1 warnings, 183 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.
"[PATCH] mesh: Combine common functions for NetKeys and AppKeys" 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.
##############################
Test: CheckGitLint - PASS
##############################
Test: CheckBuild - PASS
##############################
Test: MakeCheck - PASS
---
Regards,
Linux Bluetooth
Hi Inga,
On Tue, 2021-02-16 at 12:52 -0800, Inga Stotland wrote:
> No change in functioanality, code tightening.
> ---
> mesh/keyring.c | 120 +++++++++++++++++++++----------------------------
> 1 file changed, 50 insertions(+), 70 deletions(-)
>
> diff --git a/mesh/keyring.c b/mesh/keyring.c
> index 0b74ee914..025703574 100644
> --- a/mesh/keyring.c
> +++ b/mesh/keyring.c
> @@ -33,31 +33,45 @@ const char *dev_key_dir = "/dev_keys";
> const char *app_key_dir = "/app_keys";
> const char *net_key_dir = "/net_keys";
>
> -bool keyring_put_net_key(struct mesh_node *node, uint16_t net_idx,
> - struct keyring_net_key *key)
> +static int open_key_file(struct mesh_node *node, const char *key_dir,
> + uint16_t idx, bool is_wrt, int flags)
> {
> const char *node_path;
> - char key_file[PATH_MAX];
> - bool result = false;
> - int fd;
> + char fname[PATH_MAX];
>
> - if (!node || !key)
> - return false;
> + if (!node)
> + return -1;
>
> node_path = node_get_storage_dir(node);
>
> - if (strlen(node_path) + strlen(net_key_dir) + 1 + 3 >= PATH_MAX)
> - return false;
> + if (strlen(node_path) + strlen(key_dir) + 1 + 3 >= PATH_MAX)
> + return -1;
>
> - snprintf(key_file, PATH_MAX, "%s%s", node_path, net_key_dir);
> + if (is_wrt) {
> + snprintf(fname, PATH_MAX, "%s%s", node_path, key_dir);
> + mkdir(fname, 0755);
> + }
>
> - mkdir(key_file, 0755);
> + snprintf(fname, PATH_MAX, "%s%s/%3.3x", node_path, key_dir, idx);
>
> - snprintf(key_file, PATH_MAX, "%s%s/%3.3x", node_path, net_key_dir,
> - net_idx);
> - l_debug("Put Net Key %s", key_file);
> + if (is_wrt)
> + return open(fname, flags, S_IRUSR | S_IWUSR);
> + else
> + return open(fname, flags);
> +}
> +
> +bool keyring_put_net_key(struct mesh_node *node, uint16_t net_idx,
> + struct keyring_net_key *key)
> +{
> + bool result = false;
> + int fd;
> +
> + if (!key)
> + return false;
> +
> + fd = open_key_file(node, net_key_dir, net_idx, true,
> + O_WRONLY | O_CREAT | O_TRUNC);
>
> - fd = open(key_file, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR);
> if (fd < 0)
> return false;
>
> @@ -72,28 +86,14 @@ bool keyring_put_net_key(struct mesh_node *node, uint16_t net_idx,
> bool keyring_put_app_key(struct mesh_node *node, uint16_t app_idx,
> uint16_t net_idx, struct keyring_app_key *key)
> {
> - const char *node_path;
> - char key_file[PATH_MAX];
> bool result = false;
> int fd;
>
> - if (!node || !key)
> + if (!key)
> return false;
>
> - node_path = node_get_storage_dir(node);
> -
> - if (strlen(node_path) + strlen(app_key_dir) + 1 + 3 >= PATH_MAX)
> - return false;
> + fd = open_key_file(node, app_key_dir, app_idx, false, O_RDWR);
keyring_put_app_key writes to the file, but you are setting is_wrt to false. Is that what you want? Or maybe,
since this matches the earlier functionaility, "is_wrt" should actually be "is_create"... Since we aren't
*creating* the file, the S_IRUSR | S_IWUSR permissions do not need to be set.
>
> - snprintf(key_file, PATH_MAX, "%s%s", node_path, app_key_dir);
> -
> - mkdir(key_file, 0755);
> -
> - snprintf(key_file, PATH_MAX, "%s%s/%3.3x", node_path, app_key_dir,
> - app_idx);
> - l_debug("Put App Key %s", key_file);
> -
> - fd = open(key_file, O_RDWR);
> if (fd >= 0) {
> struct keyring_app_key old_key;
>
> @@ -105,12 +105,12 @@ bool keyring_put_app_key(struct mesh_node *node, uint16_t app_idx,
> }
>
> lseek(fd, 0, SEEK_SET);
> - } else {
> - fd = open(key_file, O_WRONLY | O_CREAT | O_TRUNC,
> - S_IRUSR | S_IWUSR);
> - if (fd < 0)
> - return false;
> - }
> + } else
> + fd = open_key_file(node, app_key_dir, app_idx, true,
> + O_WRONLY | O_CREAT | O_TRUNC);
> +
> + if (fd < 0)
> + return false;
>
> if (write(fd, key, sizeof(*key)) == sizeof(*key))
> result = true;
> @@ -212,8 +212,7 @@ bool keyring_put_remote_dev_key(struct mesh_node *node, uint16_t unicast,
> dev_key_dir, unicast + i);
> l_debug("Put Dev Key %s", key_file);
>
> - fd = open(key_file, O_WRONLY | O_CREAT | O_TRUNC,
> - S_IRUSR | S_IWUSR);
> + fd = open(key_file, O_WRONLY | O_CREAT | O_TRUNC, 0600);
So are we using S_IRUSR | S_IWUSR or 0600?
> if (fd >= 0) {
> if (write(fd, dev_key, 16) != 16)
> result = false;
> @@ -226,24 +225,19 @@ bool keyring_put_remote_dev_key(struct mesh_node *node, uint16_t unicast,
> return result;
> }
>
> -bool keyring_get_net_key(struct mesh_node *node, uint16_t net_idx,
> - struct keyring_net_key *key)
> +static bool get_key(struct mesh_node *node, const char *key_dir,
> + uint16_t key_idx, void *key, ssize_t sz)
> {
> - const char *node_path;
> - char key_file[PATH_MAX];
> bool result = false;
> int fd;
>
> - if (!node || !key)
> + if (!key)
> return false;
>
> - node_path = node_get_storage_dir(node);
> - snprintf(key_file, PATH_MAX, "%s%s/%3.3x", node_path, net_key_dir,
> - net_idx);
> + fd = open_key_file(node, key_dir, key_idx, false, O_RDONLY);
>
> - fd = open(key_file, O_RDONLY);
> if (fd >= 0) {
> - if (read(fd, key, sizeof(*key)) == sizeof(*key))
> + if (read(fd, key, sz) == sz)
> result = true;
>
> close(fd);
> @@ -252,30 +246,16 @@ bool keyring_get_net_key(struct mesh_node *node, uint16_t net_idx,
> return result;
> }
>
> +bool keyring_get_net_key(struct mesh_node *node, uint16_t net_idx,
> + struct keyring_net_key *key)
> +{
> + return get_key(node, net_key_dir, net_idx, key, sizeof(*key));
> +}
> +
> bool keyring_get_app_key(struct mesh_node *node, uint16_t app_idx,
> struct keyring_app_key *key)
> {
> - const char *node_path;
> - char key_file[PATH_MAX];
> - bool result = false;
> - int fd;
> -
> - if (!node || !key)
> - return false;
> -
> - node_path = node_get_storage_dir(node);
> - snprintf(key_file, PATH_MAX, "%s%s/%3.3x", node_path, app_key_dir,
> - app_idx);
> -
> - fd = open(key_file, O_RDONLY);
> - if (fd >= 0) {
> - if (read(fd, key, sizeof(*key)) == sizeof(*key))
> - result = true;
> -
> - close(fd);
> - }
> -
> - return result;
> + return get_key(node, app_key_dir, app_idx, key, sizeof(*key));
> }
>
> bool keyring_get_remote_dev_key(struct mesh_node *node, uint16_t unicast,