[PATCH conntrack-tools v2 0/3] fix potential memory loss and exit codes

[PATCH conntrack-tools v2 0/3] fix potential memory loss and exit codes

[Donald Yandt]

Vector data will be lost if reallocation fails, leading to undefined behaviour. 
Additionally, the indices of the allocated vector data can be represented more
precisely by using size_t as the index type.

If no configuration file or an invalid parameter is provided, the daemon should exit with
a failure status.

v2:
 - Moved variable declaration and described usage of size_t as suggested by Pablo Neira Ayuso <pablo@netfilter.org>

Donald Yandt (3):
  conntrackd: prevent memory loss if reallocation fails
  conntrackd: use size_t for element indices
  conntrackd: exit with failure status

 src/main.c   |  5 ++---
 src/vector.c | 11 ++++++-----
 2 files changed, 8 insertions(+), 8 deletions(-)

-- 
2.44.0

[PATCH conntrack-tools v2 1/3] conntrackd: prevent memory loss if reallocation fails

[Donald Yandt]

Signed-off-by: Donald Yandt <donald.yandt@gmail.com>
---
 src/vector.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/vector.c b/src/vector.c
index c81e7ce..0af8db7 100644
--- a/src/vector.c
+++ b/src/vector.c
@@ -60,13 +60,16 @@ void vector_destroy(struct vector *v)
 
 int vector_add(struct vector *v, void *data)
 {
+	void *ptr;
+
 	if (v->cur_elems >= v->max_elems) {
 		v->max_elems += DEFAULT_VECTOR_GROWTH;
-		v->data = realloc(v->data, v->max_elems * v->size);
-		if (v->data == NULL) {
+		ptr = realloc(v->data, v->max_elems * v->size);
+		if (ptr == NULL) {
 			v->max_elems -= DEFAULT_VECTOR_GROWTH;
 			return -1;
 		}
+		v->data = ptr;
 	}
 	memcpy(v->data + (v->size * v->cur_elems), data, v->size);
 	v->cur_elems++;
-- 
2.44.0

[PATCH conntrack-tools v2 2/3] conntrackd: use size_t for element indices

[Donald Yandt]

The indices of the allocated vector data can be represented more
precisely by using size_t as the index type. The size_t type integer
is used in memory allocation routines and is capable of handling any
allocated object size or index.

Signed-off-by: Donald Yandt <donald.yandt@gmail.com>
---
 src/vector.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/vector.c b/src/vector.c
index 0af8db7..fb1014f 100644
--- a/src/vector.c
+++ b/src/vector.c
@@ -23,9 +23,7 @@
 
 struct vector {
 	char *data;
-	unsigned int cur_elems;
-	unsigned int max_elems;
-	size_t size;
+	size_t cur_elems, max_elems, size;
 };
 
 #define DEFAULT_VECTOR_MEMBERS	8
-- 
2.44.0

[PATCH conntrack-tools v2 3/3] conntrackd: exit with failure status

[Donald Yandt]

Signed-off-by: Donald Yandt <donald.yandt@gmail.com>
---
 src/main.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/main.c b/src/main.c
index de4773d..c6b2600 100644
--- a/src/main.c
+++ b/src/main.c
@@ -175,7 +175,7 @@ int main(int argc, char *argv[])
 			}
 			show_usage(argv[0]);
 			dlog(LOG_ERR, "Missing config filename");
-			break;
+			exit(EXIT_FAILURE);
 		case 'F':
 			set_operation_mode(&type, REQUEST, argv);
 			i = set_action_by_table(i, argc, argv,
@@ -309,8 +309,7 @@ int main(int argc, char *argv[])
 		default:
 			show_usage(argv[0]);
 			dlog(LOG_ERR, "Unknown option: %s", argv[i]);
-			return 0;
-			break;
+			exit(EXIT_FAILURE);
 		}
 	}
 
-- 
2.44.0

Re: [PATCH conntrack-tools v2 0/3] fix potential memory loss and exit codes

[Pablo Neira Ayuso]

On Sat, Mar 02, 2024 at 11:07:59AM -0500, Donald Yandt wrote:
> Vector data will be lost if reallocation fails, leading to undefined behaviour. 
> Additionally, the indices of the allocated vector data can be represented more
> precisely by using size_t as the index type.
> 
> If no configuration file or an invalid parameter is provided, the daemon should exit with
> a failure status.

Applied.

I move this description chunks where they belong to

Re: [PATCH conntrack-tools v2 0/3] fix potential memory loss and exit codes

[Pablo Neira Ayuso]

On Mon, Mar 04, 2024 at 01:49:39PM +0100, Pablo Neira Ayuso wrote:
> On Sat, Mar 02, 2024 at 11:07:59AM -0500, Donald Yandt wrote:
> > Vector data will be lost if reallocation fails, leading to undefined behaviour. 
> > Additionally, the indices of the allocated vector data can be represented more
> > precisely by using size_t as the index type.
> > 
> > If no configuration file or an invalid parameter is provided, the daemon should exit with
> > a failure status.
> 
> Applied.

BTW, I skipped 2/3, I am not convinced this gives us much.

> I move this description chunks where they belong to

Re: [PATCH conntrack-tools v2 0/3] fix potential memory loss and exit codes

[Donald Yandt]

On Mon, Mar 4, 2024 at 7:58 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> BTW, I skipped 2/3, I am not convinced this gives us much.
>

Hi Pablo,

Thank you, I greatly appreciate you accepting patches 1/3 and 3/3. In
regards to 2/3, since we're using max_elems in allocation calls that
take size_t as a parameter,
 such as in realloc(..., v->max_elems * v->size), and cur_elems as an
offset of allocated memory, we should utilize that type. In the event
that you concur,
it would also be necessary to modify the index type on line 83, which
was missed.

Donald