You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/04/28 07:33:40 UTC
[GitHub] [incubator-nuttx] realprocrastinator opened a new pull request, #6171: syslog: Fixed a potential buffer overflow issue
realprocrastinator opened a new pull request, #6171:
URL: https://github.com/apache/incubator-nuttx/pull/6171
## Summary
To avoid a potential buffer overflow issue by compile-time sanity check.
## Impact
The `CONFIG_SYSLOG_MAX_CHANNELS` is user-specified, however, if the user
defines more channels than what `CONFIG_SYSLOG_MAX_CHANNELS` was defined as,
a potential buffer overflow may occur. Although the compiler does warn us about
that, we should explicitly tell the user this is an error.
NOTE: This is a simple method that relies on the Kconfig to convert a bool type symbol to
a numeric value, `y` to `1`. And according to what the C standard specifies, all remaining
identifies are replaced with `pp-number 0` at preprocessing phase. Hence this should work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6171: syslog: Fixed a potential buffer overflow issue
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6171:
URL: https://github.com/apache/incubator-nuttx/pull/6171#discussion_r867334156
##########
include/nuttx/nuttx.h:
##########
@@ -48,4 +48,20 @@
#define container_of(ptr, type, member) \
((type *)((uintptr_t)(ptr) - offsetof(type, member)))
+/* Name: is_config_set
+ *
+ * Description:
+ * Evaluate a kconfig-provided boolen type configuration setting
+ * at compile time
+ *
+ * Arguments:
+ * config - The configuration setting name.
+ */
+
+#define is_config_set(config) _is_config_set(config)
Review Comment:
can we simplify the check?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6171: syslog: Fixed a potential buffer overflow issue
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6171:
URL: https://github.com/apache/incubator-nuttx/pull/6171#discussion_r860637268
##########
drivers/syslog/syslog_channel.c:
##########
@@ -129,6 +129,16 @@ static struct syslog_channel_s g_default_channel =
};
#endif
+/* This is a simply sanity check to avoid we have more elements than the
+ * `g_syslog_channel` array can hold
+ */
+
+#if (CONFIG_SYSLOG_DEFAULT + CONFIG_RAMLOG_SYSLOG + \
+ CONFIG_SYSLOG_RPMSG + CONFIG_SYSLOG_RTT) > \
+ CONFIG_SYSLOG_MAX_CHANNELS
+# error "Max syslog channel number exceeds"
Review Comment:
think this will generate warnings if any of `CONFIG_SYSLOG_DEFAULT`, `CONFIG_RAMLOG_SYSLOG`, `CONFIG_SYSLOG_RPMSG` or `CONFIG_SYSLOG_RTT` are not defined.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] realprocrastinator commented on a diff in pull request #6171: syslog: Fixed a potential buffer overflow issue
Posted by GitBox <gi...@apache.org>.
realprocrastinator commented on code in PR #6171:
URL: https://github.com/apache/incubator-nuttx/pull/6171#discussion_r874442039
##########
include/nuttx/nuttx.h:
##########
@@ -48,4 +48,20 @@
#define container_of(ptr, type, member) \
((type *)((uintptr_t)(ptr) - offsetof(type, member)))
+/* Name: is_config_set
+ *
+ * Description:
+ * Evaluate a kconfig-provided boolen type configuration setting
+ * at compile time
+ *
+ * Arguments:
+ * config - The configuration setting name.
+ */
+
+#define is_config_set(config) _is_config_set(config)
Review Comment:
> Maybe we can add in `drivers/syslog/syslog_channel.c` something like:
>
> ```
> #ifdef CONFIG_SYSLOG_DEFAULT
> # define SYSLOG_DEFAULT_AVAILABLE 1
> #else
> # define SYSLOG_DEFAULT_AVAILABLE 0
> #endif
>
> #ifdef CONFIG_RAMLOG_SYSLOG
> # define RAMLOG_SYSLOG_AVAILABLE 1
> #else
> # define RAMLOG_SYSLOG_AVAILABLE 0
> #endif
>
> #ifdef CONFIG_SYSLOG_RPMSG
> # define SYSLOG_RPMSG_AVAILABLE 1
> #else
> # define SYSLOG_RPMSG_AVAILABLE 0
> #endif
>
> #ifdef CONFIG_SYSLOG_RTT
> # define SYSLOG_RTT_AVAILABLE 1
> #else
> # define SYSLOG_RTT_AVAILABLE 0
> #endif
>
> #define SYSLOG_NCHANNELS (SYSLOG_DEFAULT_AVAILABLE + \
> RAMLOG_SYSLOG_AVAILABLE + \
> SYSLOG_RPMSG_AVAILABLE + \
> SYSLOG_RTT_AVAILABLE)
> ```
>
> ?? Still some more code, but is simple that should fit to solve the issue.
>
> Then I think we even should not need a static assert, but can
>
> ```
> #if SYSLOG_NCHANNELS > CONFIG_SYSLOG_MAX_CHANNELS
> # error "Maximum channel number exceeds."
> #endif
> ```
Oh I see. That's quite straightforward, will change to that instead. Thx!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6171: syslog: Fixed a potential buffer overflow issue
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6171:
URL: https://github.com/apache/incubator-nuttx/pull/6171#discussion_r867500365
##########
include/nuttx/nuttx.h:
##########
@@ -48,4 +48,20 @@
#define container_of(ptr, type, member) \
((type *)((uintptr_t)(ptr) - offsetof(type, member)))
+/* Name: is_config_set
+ *
+ * Description:
+ * Evaluate a kconfig-provided boolen type configuration setting
+ * at compile time
+ *
+ * Arguments:
+ * config - The configuration setting name.
+ */
+
+#define is_config_set(config) _is_config_set(config)
Review Comment:
Maybe we can add in `drivers/syslog/syslog_channel.c` something like:
```
#ifdef CONFIG_SYSLOG_DEFAULT
# define SYSLOG_DEFAULT_AVAILABLE 1
#else
# define SYSLOG_DEFAULT_AVAILABLE 0
#endif
#ifdef CONFIG_RAMLOG_SYSLOG
# define RAMLOG_SYSLOG_AVAILABLE 1
#else
# define RAMLOG_SYSLOG_AVAILABLE 0
#endif
#ifdef CONFIG_SYSLOG_RPMSG
# define SYSLOG_RPMSG_AVAILABLE 1
#else
# define SYSLOG_RPMSG_AVAILABLE 0
#endif
#ifdef CONFIG_SYSLOG_RTT
# define SYSLOG_RTT_AVAILABLE 1
#else
# define SYSLOG_RTT_AVAILABLE 0
#endif
#define SYSLOG_NCHANNELS (SYSLOG_DEFAULT_AVAILABLE +\
CONFIG_RAMLOG_SYSLOG + \
CONFIG_SYSLOG_RPMSG + \
SYSLOG_RTT_AVAILABLE)
```
??
Still some more code, but is simple that should fit to solve the issue.
##########
include/nuttx/nuttx.h:
##########
@@ -48,4 +48,20 @@
#define container_of(ptr, type, member) \
((type *)((uintptr_t)(ptr) - offsetof(type, member)))
+/* Name: is_config_set
+ *
+ * Description:
+ * Evaluate a kconfig-provided boolen type configuration setting
+ * at compile time
+ *
+ * Arguments:
+ * config - The configuration setting name.
+ */
+
+#define is_config_set(config) _is_config_set(config)
Review Comment:
Maybe we can add in `drivers/syslog/syslog_channel.c` something like:
```
#ifdef CONFIG_SYSLOG_DEFAULT
# define SYSLOG_DEFAULT_AVAILABLE 1
#else
# define SYSLOG_DEFAULT_AVAILABLE 0
#endif
#ifdef CONFIG_RAMLOG_SYSLOG
# define RAMLOG_SYSLOG_AVAILABLE 1
#else
# define RAMLOG_SYSLOG_AVAILABLE 0
#endif
#ifdef CONFIG_SYSLOG_RPMSG
# define SYSLOG_RPMSG_AVAILABLE 1
#else
# define SYSLOG_RPMSG_AVAILABLE 0
#endif
#ifdef CONFIG_SYSLOG_RTT
# define SYSLOG_RTT_AVAILABLE 1
#else
# define SYSLOG_RTT_AVAILABLE 0
#endif
#define SYSLOG_NCHANNELS (SYSLOG_DEFAULT_AVAILABLE +\
CONFIG_RAMLOG_SYSLOG + \
CONFIG_SYSLOG_RPMSG + \
SYSLOG_RTT_AVAILABLE)
```
??
Still some more code, but is simple that should fit to solve the issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6171: syslog: Fixed a potential buffer overflow issue
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6171:
URL: https://github.com/apache/incubator-nuttx/pull/6171#discussion_r867500365
##########
include/nuttx/nuttx.h:
##########
@@ -48,4 +48,20 @@
#define container_of(ptr, type, member) \
((type *)((uintptr_t)(ptr) - offsetof(type, member)))
+/* Name: is_config_set
+ *
+ * Description:
+ * Evaluate a kconfig-provided boolen type configuration setting
+ * at compile time
+ *
+ * Arguments:
+ * config - The configuration setting name.
+ */
+
+#define is_config_set(config) _is_config_set(config)
Review Comment:
Maybe we can add in `drivers/syslog/syslog_channel.c` something like:
```
#ifdef CONFIG_SYSLOG_DEFAULT
# define SYSLOG_DEFAULT_AVAILABLE 1
#else
# define SYSLOG_DEFAULT_AVAILABLE 0
#endif
#ifdef CONFIG_RAMLOG_SYSLOG
# define RAMLOG_SYSLOG_AVAILABLE 1
#else
# define RAMLOG_SYSLOG_AVAILABLE 0
#endif
#ifdef CONFIG_SYSLOG_RPMSG
# define SYSLOG_RPMSG_AVAILABLE 1
#else
# define SYSLOG_RPMSG_AVAILABLE 0
#endif
#ifdef CONFIG_SYSLOG_RTT
# define SYSLOG_RTT_AVAILABLE 1
#else
# define SYSLOG_RTT_AVAILABLE 0
#endif
#define SYSLOG_NCHANNELS (SYSLOG_DEFAULT_AVAILABLE + \
RAMLOG_SYSLOG_AVAILABLE + \
SYSLOG_RPMSG_AVAILABLE + \
SYSLOG_RTT_AVAILABLE)
```
??
Still some more code, but is simple that should fit to solve the issue.
Then I think we even should not need a static assert, but can
```
#if SYSLOG_NCHANNELS > CONFIG_SYSLOG_MAX_CHANNELS
# error "Maximum channel number exceeds."
#endif
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] realprocrastinator commented on a diff in pull request #6171: syslog: Fixed a potential buffer overflow issue
Posted by GitBox <gi...@apache.org>.
realprocrastinator commented on code in PR #6171:
URL: https://github.com/apache/incubator-nuttx/pull/6171#discussion_r867429922
##########
include/nuttx/nuttx.h:
##########
@@ -48,4 +48,20 @@
#define container_of(ptr, type, member) \
((type *)((uintptr_t)(ptr) - offsetof(type, member)))
+/* Name: is_config_set
+ *
+ * Description:
+ * Evaluate a kconfig-provided boolen type configuration setting
+ * at compile time
+ *
+ * Arguments:
+ * config - The configuration setting name.
+ */
+
+#define is_config_set(config) _is_config_set(config)
Review Comment:
> I mean the macro self, it's specially vararg in _is_config_set_evaluate.
Right, I agree this is a bit hacky, lemme think of some other ways.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6171: syslog: Fixed a potential buffer overflow issue
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6171:
URL: https://github.com/apache/incubator-nuttx/pull/6171#discussion_r867500365
##########
include/nuttx/nuttx.h:
##########
@@ -48,4 +48,20 @@
#define container_of(ptr, type, member) \
((type *)((uintptr_t)(ptr) - offsetof(type, member)))
+/* Name: is_config_set
+ *
+ * Description:
+ * Evaluate a kconfig-provided boolen type configuration setting
+ * at compile time
+ *
+ * Arguments:
+ * config - The configuration setting name.
+ */
+
+#define is_config_set(config) _is_config_set(config)
Review Comment:
Maybe we can add in `drivers/syslog/syslog_channel.c` something like:
```
#ifdef CONFIG_SYSLOG_DEFAULT
# define SYSLOG_DEFAULT_AVAILABLE 1
#else
# define SYSLOG_DEFAULT_AVAILABLE 0
#endif
#ifdef CONFIG_RAMLOG_SYSLOG
# define RAMLOG_SYSLOG_AVAILABLE 1
#else
# define RAMLOG_SYSLOG_AVAILABLE 0
#endif
#ifdef CONFIG_SYSLOG_RPMSG
# define SYSLOG_RPMSG_AVAILABLE 1
#else
# define SYSLOG_RPMSG_AVAILABLE 0
#endif
#ifdef CONFIG_SYSLOG_RTT
# define SYSLOG_RTT_AVAILABLE 1
#else
# define SYSLOG_RTT_AVAILABLE 0
#endif
#define SYSLOG_NCHANNELS (SYSLOG_DEFAULT_AVAILABLE +\
CONFIG_RAMLOG_SYSLOG + \
CONFIG_SYSLOG_RPMSG + \
SYSLOG_RTT_AVAILABLE)
```
??
Still some more code, but is simple that should fit to solve the issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] realprocrastinator commented on a diff in pull request #6171: syslog: Fixed a potential buffer overflow issue
Posted by GitBox <gi...@apache.org>.
realprocrastinator commented on code in PR #6171:
URL: https://github.com/apache/incubator-nuttx/pull/6171#discussion_r860748241
##########
drivers/syslog/syslog_channel.c:
##########
@@ -129,6 +129,16 @@ static struct syslog_channel_s g_default_channel =
};
#endif
+/* This is a simply sanity check to avoid we have more elements than the
+ * `g_syslog_channel` array can hold
+ */
+
+#if (CONFIG_SYSLOG_DEFAULT + CONFIG_RAMLOG_SYSLOG + \
+ CONFIG_SYSLOG_RPMSG + CONFIG_SYSLOG_RTT) > \
+ CONFIG_SYSLOG_MAX_CHANNELS
+# error "Max syslog channel number exceeds"
Review Comment:
Sure, will have a look at changing to `static_assert` method. :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] realprocrastinator commented on a diff in pull request #6171: syslog: Fixed a potential buffer overflow issue
Posted by GitBox <gi...@apache.org>.
realprocrastinator commented on code in PR #6171:
URL: https://github.com/apache/incubator-nuttx/pull/6171#discussion_r860751365
##########
drivers/syslog/syslog_channel.c:
##########
@@ -129,6 +129,16 @@ static struct syslog_channel_s g_default_channel =
};
#endif
+/* This is a simply sanity check to avoid we have more elements than the
+ * `g_syslog_channel` array can hold
+ */
+
+#if (CONFIG_SYSLOG_DEFAULT + CONFIG_RAMLOG_SYSLOG + \
+ CONFIG_SYSLOG_RPMSG + CONFIG_SYSLOG_RTT) > \
+ CONFIG_SYSLOG_MAX_CHANNELS
+# error "Max syslog channel number exceeds"
Review Comment:
> think this will generate warnings if any of `CONFIG_SYSLOG_DEFAULT`, `CONFIG_RAMLOG_SYSLOG`, `CONFIG_SYSLOG_RPMSG` or `CONFIG_SYSLOG_RTT` are not defined.
True, will have a think about a better solution. Thx!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6171: syslog: Fixed a potential buffer overflow issue
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6171:
URL: https://github.com/apache/incubator-nuttx/pull/6171#discussion_r860610396
##########
drivers/syslog/syslog_channel.c:
##########
@@ -129,6 +129,16 @@ static struct syslog_channel_s g_default_channel =
};
#endif
+/* This is a simply sanity check to avoid we have more elements than the
+ * `g_syslog_channel` array can hold
+ */
+
+#if (CONFIG_SYSLOG_DEFAULT + CONFIG_RAMLOG_SYSLOG + \
+ CONFIG_SYSLOG_RPMSG + CONFIG_SYSLOG_RTT) > \
+ CONFIG_SYSLOG_MAX_CHANNELS
+# error "Max syslog channel number exceeds"
Review Comment:
how about change to static_assert?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] realprocrastinator commented on a diff in pull request #6171: syslog: Fixed a potential buffer overflow issue
Posted by GitBox <gi...@apache.org>.
realprocrastinator commented on code in PR #6171:
URL: https://github.com/apache/incubator-nuttx/pull/6171#discussion_r860863937
##########
drivers/syslog/syslog_channel.c:
##########
@@ -129,6 +129,16 @@ static struct syslog_channel_s g_default_channel =
};
#endif
+/* This is a simply sanity check to avoid we have more elements than the
+ * `g_syslog_channel` array can hold
+ */
+
+#if (CONFIG_SYSLOG_DEFAULT + CONFIG_RAMLOG_SYSLOG + \
+ CONFIG_SYSLOG_RPMSG + CONFIG_SYSLOG_RTT) > \
+ CONFIG_SYSLOG_MAX_CHANNELS
+# error "Max syslog channel number exceeds"
Review Comment:
Another question: is it a good idea to let `SYSLOG_MAX_CHANNELS` be determined automatically by filling `CONFIG_SYSLOG_DEFAULT`, `CONFIG_RAMLOG_SYSLOG`, etc. into an `enum` data type?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6171: syslog: Fixed a potential buffer overflow issue
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6171:
URL: https://github.com/apache/incubator-nuttx/pull/6171#discussion_r867347582
##########
include/nuttx/nuttx.h:
##########
@@ -48,4 +48,20 @@
#define container_of(ptr, type, member) \
((type *)((uintptr_t)(ptr) - offsetof(type, member)))
+/* Name: is_config_set
+ *
+ * Description:
+ * Evaluate a kconfig-provided boolen type configuration setting
+ * at compile time
+ *
+ * Arguments:
+ * config - The configuration setting name.
+ */
+
+#define is_config_set(config) _is_config_set(config)
Review Comment:
I mean the macro self, it's specially vararg in _is_config_set_evaluate.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] realprocrastinator commented on a diff in pull request #6171: syslog: Fixed a potential buffer overflow issue
Posted by GitBox <gi...@apache.org>.
realprocrastinator commented on code in PR #6171:
URL: https://github.com/apache/incubator-nuttx/pull/6171#discussion_r861419731
##########
drivers/syslog/syslog_channel.c:
##########
@@ -129,6 +129,16 @@ static struct syslog_channel_s g_default_channel =
};
#endif
+/* This is a simply sanity check to avoid we have more elements than the
+ * `g_syslog_channel` array can hold
+ */
+
+#if (CONFIG_SYSLOG_DEFAULT + CONFIG_RAMLOG_SYSLOG + \
+ CONFIG_SYSLOG_RPMSG + CONFIG_SYSLOG_RTT) > \
+ CONFIG_SYSLOG_MAX_CHANNELS
+# error "Max syslog channel number exceeds"
Review Comment:
> It doesn't work as expect, because the user may add additional channel dynamically.
I see, thx!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6171: syslog: Fixed a potential buffer overflow issue
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6171:
URL: https://github.com/apache/incubator-nuttx/pull/6171#discussion_r867500365
##########
include/nuttx/nuttx.h:
##########
@@ -48,4 +48,20 @@
#define container_of(ptr, type, member) \
((type *)((uintptr_t)(ptr) - offsetof(type, member)))
+/* Name: is_config_set
+ *
+ * Description:
+ * Evaluate a kconfig-provided boolen type configuration setting
+ * at compile time
+ *
+ * Arguments:
+ * config - The configuration setting name.
+ */
+
+#define is_config_set(config) _is_config_set(config)
Review Comment:
Maybe we can add in `drivers/syslog/syslog_channel.c` something like:
```
#ifdef CONFIG_SYSLOG_DEFAULT
# define SYSLOG_DEFAULT_AVAILABLE 1
#else
# define SYSLOG_DEFAULT_AVAILABLE 0
#endif
#ifdef CONFIG_RAMLOG_SYSLOG
# define RAMLOG_SYSLOG_AVAILABLE 1
#else
# define RAMLOG_SYSLOG_AVAILABLE 0
#endif
#ifdef CONFIG_SYSLOG_RPMSG
# define SYSLOG_RPMSG_AVAILABLE 1
#else
# define SYSLOG_RPMSG_AVAILABLE 0
#endif
#ifdef CONFIG_SYSLOG_RTT
# define SYSLOG_RTT_AVAILABLE 1
#else
# define SYSLOG_RTT_AVAILABLE 0
#endif
#define SYSLOG_NCHANNELS (SYSLOG_DEFAULT_AVAILABLE + \
RAMLOG_SYSLOG_AVAILABLE + \
SYSLOG_RPMSG_AVAILABLE + \
SYSLOG_RTT_AVAILABLE)
```
??
Still some more code, but is simple that should fit to solve the issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6171: syslog: Fixed a potential buffer overflow issue
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6171:
URL: https://github.com/apache/incubator-nuttx/pull/6171#discussion_r860877084
##########
drivers/syslog/syslog_channel.c:
##########
@@ -129,6 +129,16 @@ static struct syslog_channel_s g_default_channel =
};
#endif
+/* This is a simply sanity check to avoid we have more elements than the
+ * `g_syslog_channel` array can hold
+ */
+
+#if (CONFIG_SYSLOG_DEFAULT + CONFIG_RAMLOG_SYSLOG + \
+ CONFIG_SYSLOG_RPMSG + CONFIG_SYSLOG_RTT) > \
+ CONFIG_SYSLOG_MAX_CHANNELS
+# error "Max syslog channel number exceeds"
Review Comment:
It doesn't work as expect, because the user may add additional channel dynamically.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #6171: syslog: Fixed a potential buffer overflow issue
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6171:
URL: https://github.com/apache/incubator-nuttx/pull/6171
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] realprocrastinator commented on a diff in pull request #6171: syslog: Fixed a potential buffer overflow issue
Posted by GitBox <gi...@apache.org>.
realprocrastinator commented on code in PR #6171:
URL: https://github.com/apache/incubator-nuttx/pull/6171#discussion_r867340916
##########
include/nuttx/nuttx.h:
##########
@@ -48,4 +48,20 @@
#define container_of(ptr, type, member) \
((type *)((uintptr_t)(ptr) - offsetof(type, member)))
+/* Name: is_config_set
+ *
+ * Description:
+ * Evaluate a kconfig-provided boolen type configuration setting
+ * at compile time
+ *
+ * Arguments:
+ * config - The configuration setting name.
+ */
+
+#define is_config_set(config) _is_config_set(config)
Review Comment:
Shall I simplify this macro or simplify the whole sanity check method? Could you give me some more hints pls?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6171: syslog: Fixed a potential buffer overflow issue
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6171:
URL: https://github.com/apache/incubator-nuttx/pull/6171#discussion_r867374192
##########
include/nuttx/nuttx.h:
##########
@@ -48,4 +48,20 @@
#define container_of(ptr, type, member) \
((type *)((uintptr_t)(ptr) - offsetof(type, member)))
+/* Name: is_config_set
+ *
+ * Description:
+ * Evaluate a kconfig-provided boolen type configuration setting
+ * at compile time
+ *
+ * Arguments:
+ * config - The configuration setting name.
+ */
+
+#define is_config_set(config) _is_config_set(config)
Review Comment:
Let me think a little a bit about it as well
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org