linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] xdp_rxq_info_user: Replace malloc/memset w/calloc
@ 2020-06-11 15:02 Gaurav Singh
  2020-06-11 18:01 ` Jesper Dangaard Brouer
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Gaurav Singh @ 2020-06-11 15:02 UTC (permalink / raw)
  To: gaurav1086, Alexei Starovoitov, Daniel Borkmann, David S. Miller,
	Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend,
	Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
	KP Singh, open list:XDP (eXpress Data Path),
	open list:XDP (eXpress Data Path),
	open list

Replace malloc/memset with calloc

Signed-off-by: Gaurav Singh <gaurav1086@gmail.com>
---
 samples/bpf/xdp_rxq_info_user.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c
index 4fe47502ebed..caa4e7ffcfc7 100644
--- a/samples/bpf/xdp_rxq_info_user.c
+++ b/samples/bpf/xdp_rxq_info_user.c
@@ -198,11 +198,8 @@ static struct datarec *alloc_record_per_cpu(void)
 {
 	unsigned int nr_cpus = bpf_num_possible_cpus();
 	struct datarec *array;
-	size_t size;
 
-	size = sizeof(struct datarec) * nr_cpus;
-	array = malloc(size);
-	memset(array, 0, size);
+	array = calloc(nr_cpus, sizeof(struct datarec));
 	if (!array) {
 		fprintf(stderr, "Mem alloc error (nr_cpus:%u)\n", nr_cpus);
 		exit(EXIT_FAIL_MEM);
@@ -214,11 +211,8 @@ static struct record *alloc_record_per_rxq(void)
 {
 	unsigned int nr_rxqs = bpf_map__def(rx_queue_index_map)->max_entries;
 	struct record *array;
-	size_t size;
 
-	size = sizeof(struct record) * nr_rxqs;
-	array = malloc(size);
-	memset(array, 0, size);
+	array = calloc(nr_rxqs, sizeof(struct record));
 	if (!array) {
 		fprintf(stderr, "Mem alloc error (nr_rxqs:%u)\n", nr_rxqs);
 		exit(EXIT_FAIL_MEM);
@@ -232,8 +226,7 @@ static struct stats_record *alloc_stats_record(void)
 	struct stats_record *rec;
 	int i;
 
-	rec = malloc(sizeof(*rec));
-	memset(rec, 0, sizeof(*rec));
+	rec = calloc(1, sizeof(struct stats_record));
 	if (!rec) {
 		fprintf(stderr, "Mem alloc error\n");
 		exit(EXIT_FAIL_MEM);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] xdp_rxq_info_user: Replace malloc/memset w/calloc
  2020-06-11 15:02 [PATCH] xdp_rxq_info_user: Replace malloc/memset w/calloc Gaurav Singh
@ 2020-06-11 18:01 ` Jesper Dangaard Brouer
  2020-06-12  0:26 ` Gaurav Singh
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Jesper Dangaard Brouer @ 2020-06-11 18:01 UTC (permalink / raw)
  To: Gaurav Singh
  Cc: Alexei Starovoitov, Daniel Borkmann, David S. Miller,
	Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend,
	Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
	KP Singh, open list:XDP (eXpress Data Path),
	open list:XDP (eXpress Data Path),
	open list

On Thu, 11 Jun 2020 11:02:21 -0400
Gaurav Singh <gaurav1086@gmail.com> wrote:

> Replace malloc/memset with calloc

Please also mention/describe  that this also solves the bug you found.

As this fix a potential bug, it will be appropriate to add a "Fixes:"
line, just before "Signed-off-by" (meaning no newline between the two).

Fixes: 0fca931a6f21 ("samples/bpf: program demonstrating access to xdp_rxq_info")
> Signed-off-by: Gaurav Singh <gaurav1086@gmail.com>
> ---
>  samples/bpf/xdp_rxq_info_user.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c
> index 4fe47502ebed..caa4e7ffcfc7 100644
> --- a/samples/bpf/xdp_rxq_info_user.c
> +++ b/samples/bpf/xdp_rxq_info_user.c
> @@ -198,11 +198,8 @@ static struct datarec *alloc_record_per_cpu(void)
>  {
>  	unsigned int nr_cpus = bpf_num_possible_cpus();
>  	struct datarec *array;
> -	size_t size;
>  
> -	size = sizeof(struct datarec) * nr_cpus;
> -	array = malloc(size);
> -	memset(array, 0, size);
> +	array = calloc(nr_cpus, sizeof(struct datarec));
>  	if (!array) {
>  		fprintf(stderr, "Mem alloc error (nr_cpus:%u)\n", nr_cpus);
>  		exit(EXIT_FAIL_MEM);
> @@ -214,11 +211,8 @@ static struct record *alloc_record_per_rxq(void)
>  {
>  	unsigned int nr_rxqs = bpf_map__def(rx_queue_index_map)->max_entries;
>  	struct record *array;
> -	size_t size;
>  
> -	size = sizeof(struct record) * nr_rxqs;
> -	array = malloc(size);
> -	memset(array, 0, size);
> +	array = calloc(nr_rxqs, sizeof(struct record));
>  	if (!array) {
>  		fprintf(stderr, "Mem alloc error (nr_rxqs:%u)\n", nr_rxqs);
>  		exit(EXIT_FAIL_MEM);
> @@ -232,8 +226,7 @@ static struct stats_record *alloc_stats_record(void)
>  	struct stats_record *rec;
>  	int i;
>  
> -	rec = malloc(sizeof(*rec));
> -	memset(rec, 0, sizeof(*rec));
> +	rec = calloc(1, sizeof(struct stats_record));
>  	if (!rec) {
>  		fprintf(stderr, "Mem alloc error\n");
>  		exit(EXIT_FAIL_MEM);



-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH] xdp_rxq_info_user: Replace malloc/memset w/calloc
  2020-06-11 15:02 [PATCH] xdp_rxq_info_user: Replace malloc/memset w/calloc Gaurav Singh
  2020-06-11 18:01 ` Jesper Dangaard Brouer
@ 2020-06-12  0:26 ` Gaurav Singh
  2020-06-12  0:36 ` Gaurav Singh
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Gaurav Singh @ 2020-06-12  0:26 UTC (permalink / raw)
  To: gaurav1086, Alexei Starovoitov, Daniel Borkmann, David S. Miller,
	Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend,
	Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
	KP Singh, open list:XDP (eXpress Data Path),
	open list:XDP (eXpress Data Path),
	open list

Replace malloc/memset with calloc

Fixes: 0fca931a6f21 ("samples/bpf: program demonstrating access to xdp_rxq_info") Signed-off-by: Gaurav Singh <gaurav1086@gmail.com>
---
 samples/bpf/xdp_rxq_info_user.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c
index 4fe47502ebed..caa4e7ffcfc7 100644
--- a/samples/bpf/xdp_rxq_info_user.c
+++ b/samples/bpf/xdp_rxq_info_user.c
@@ -198,11 +198,8 @@ static struct datarec *alloc_record_per_cpu(void)
 {
 	unsigned int nr_cpus = bpf_num_possible_cpus();
 	struct datarec *array;
-	size_t size;
 
-	size = sizeof(struct datarec) * nr_cpus;
-	array = malloc(size);
-	memset(array, 0, size);
+	array = calloc(nr_cpus, sizeof(struct datarec));
 	if (!array) {
 		fprintf(stderr, "Mem alloc error (nr_cpus:%u)\n", nr_cpus);
 		exit(EXIT_FAIL_MEM);
@@ -214,11 +211,8 @@ static struct record *alloc_record_per_rxq(void)
 {
 	unsigned int nr_rxqs = bpf_map__def(rx_queue_index_map)->max_entries;
 	struct record *array;
-	size_t size;
 
-	size = sizeof(struct record) * nr_rxqs;
-	array = malloc(size);
-	memset(array, 0, size);
+	array = calloc(nr_rxqs, sizeof(struct record));
 	if (!array) {
 		fprintf(stderr, "Mem alloc error (nr_rxqs:%u)\n", nr_rxqs);
 		exit(EXIT_FAIL_MEM);
@@ -232,8 +226,7 @@ static struct stats_record *alloc_stats_record(void)
 	struct stats_record *rec;
 	int i;
 
-	rec = malloc(sizeof(*rec));
-	memset(rec, 0, sizeof(*rec));
+	rec = calloc(1, sizeof(struct stats_record));
 	if (!rec) {
 		fprintf(stderr, "Mem alloc error\n");
 		exit(EXIT_FAIL_MEM);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH] xdp_rxq_info_user: Replace malloc/memset w/calloc
  2020-06-11 15:02 [PATCH] xdp_rxq_info_user: Replace malloc/memset w/calloc Gaurav Singh
  2020-06-11 18:01 ` Jesper Dangaard Brouer
  2020-06-12  0:26 ` Gaurav Singh
@ 2020-06-12  0:36 ` Gaurav Singh
  2020-06-12  6:42   ` Jesper Dangaard Brouer
  2020-06-12 18:53 ` [PATCH] xdp_rxq_info_user: Fix null pointer dereference. Replace malloc/memset with calloc Gaurav Singh
  2020-06-18  0:11 ` [PATCH] ia64: Add null pointer check for task in default_handler Gaurav Singh
  4 siblings, 1 reply; 13+ messages in thread
From: Gaurav Singh @ 2020-06-12  0:36 UTC (permalink / raw)
  To: gaurav1086, Alexei Starovoitov, Daniel Borkmann, David S. Miller,
	Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend,
	Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
	KP Singh, open list:XDP (eXpress Data Path),
	open list:XDP (eXpress Data Path),
	open list

Replace malloc/memset with calloc

Fixes: 0fca931a6f21 ("samples/bpf: program demonstrating access to xdp_rxq_info")
Signed-off-by: Gaurav Singh <gaurav1086@gmail.com>
---
 samples/bpf/xdp_rxq_info_user.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c
index 4fe47502ebed..caa4e7ffcfc7 100644
--- a/samples/bpf/xdp_rxq_info_user.c
+++ b/samples/bpf/xdp_rxq_info_user.c
@@ -198,11 +198,8 @@ static struct datarec *alloc_record_per_cpu(void)
 {
 	unsigned int nr_cpus = bpf_num_possible_cpus();
 	struct datarec *array;
-	size_t size;
 
-	size = sizeof(struct datarec) * nr_cpus;
-	array = malloc(size);
-	memset(array, 0, size);
+	array = calloc(nr_cpus, sizeof(struct datarec));
 	if (!array) {
 		fprintf(stderr, "Mem alloc error (nr_cpus:%u)\n", nr_cpus);
 		exit(EXIT_FAIL_MEM);
@@ -214,11 +211,8 @@ static struct record *alloc_record_per_rxq(void)
 {
 	unsigned int nr_rxqs = bpf_map__def(rx_queue_index_map)->max_entries;
 	struct record *array;
-	size_t size;
 
-	size = sizeof(struct record) * nr_rxqs;
-	array = malloc(size);
-	memset(array, 0, size);
+	array = calloc(nr_rxqs, sizeof(struct record));
 	if (!array) {
 		fprintf(stderr, "Mem alloc error (nr_rxqs:%u)\n", nr_rxqs);
 		exit(EXIT_FAIL_MEM);
@@ -232,8 +226,7 @@ static struct stats_record *alloc_stats_record(void)
 	struct stats_record *rec;
 	int i;
 
-	rec = malloc(sizeof(*rec));
-	memset(rec, 0, sizeof(*rec));
+	rec = calloc(1, sizeof(struct stats_record));
 	if (!rec) {
 		fprintf(stderr, "Mem alloc error\n");
 		exit(EXIT_FAIL_MEM);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] xdp_rxq_info_user: Replace malloc/memset w/calloc
  2020-06-12  0:36 ` Gaurav Singh
@ 2020-06-12  6:42   ` Jesper Dangaard Brouer
  2020-06-12 10:14     ` Joe Perches
  0 siblings, 1 reply; 13+ messages in thread
From: Jesper Dangaard Brouer @ 2020-06-12  6:42 UTC (permalink / raw)
  To: Gaurav Singh
  Cc: Alexei Starovoitov, Daniel Borkmann, David S. Miller,
	Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend,
	Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
	KP Singh, open list:XDP (eXpress Data Path),
	open list:XDP (eXpress Data Path),
	open list

On Thu, 11 Jun 2020 20:36:40 -0400
Gaurav Singh <gaurav1086@gmail.com> wrote:

> Replace malloc/memset with calloc
> 
> Fixes: 0fca931a6f21 ("samples/bpf: program demonstrating access to xdp_rxq_info")
> Signed-off-by: Gaurav Singh <gaurav1086@gmail.com>

Above is the correct use of Fixes + Signed-off-by.

Now you need to update/improve the description, to also
mention/describe that this also solves the bug you found.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] xdp_rxq_info_user: Replace malloc/memset w/calloc
  2020-06-12  6:42   ` Jesper Dangaard Brouer
@ 2020-06-12 10:14     ` Joe Perches
  2020-06-12 12:05       ` Jesper Dangaard Brouer
  2020-06-12 12:06       ` Toke Høiland-Jørgensen
  0 siblings, 2 replies; 13+ messages in thread
From: Joe Perches @ 2020-06-12 10:14 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, Gaurav Singh
  Cc: Alexei Starovoitov, Daniel Borkmann, David S. Miller,
	Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend,
	Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
	KP Singh, open list:XDP (eXpress Data Path),
	open list:XDP (eXpress Data Path),
	open list

On Fri, 2020-06-12 at 08:42 +0200, Jesper Dangaard Brouer wrote:
> On Thu, 11 Jun 2020 20:36:40 -0400
> Gaurav Singh <gaurav1086@gmail.com> wrote:
> 
> > Replace malloc/memset with calloc
> > 
> > Fixes: 0fca931a6f21 ("samples/bpf: program demonstrating access to xdp_rxq_info")
> > Signed-off-by: Gaurav Singh <gaurav1086@gmail.com>
> 
> Above is the correct use of Fixes + Signed-off-by.
> 
> Now you need to update/improve the description, to also
> mention/describe that this also solves the bug you found.

This is not a fix, it's a conversion of one
correct code to a shorter one.



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] xdp_rxq_info_user: Replace malloc/memset w/calloc
  2020-06-12 10:14     ` Joe Perches
@ 2020-06-12 12:05       ` Jesper Dangaard Brouer
  2020-06-12 15:19         ` Joe Perches
  2020-06-12 12:06       ` Toke Høiland-Jørgensen
  1 sibling, 1 reply; 13+ messages in thread
From: Jesper Dangaard Brouer @ 2020-06-12 12:05 UTC (permalink / raw)
  To: Joe Perches
  Cc: Gaurav Singh, Alexei Starovoitov, Daniel Borkmann,
	David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
	John Fastabend, Martin KaFai Lau, Song Liu, Yonghong Song,
	Andrii Nakryiko, KP Singh, open list:XDP (eXpress Data Path),
	open list:XDP (eXpress Data Path),
	open list

On Fri, 12 Jun 2020 03:14:58 -0700
Joe Perches <joe@perches.com> wrote:

> On Fri, 2020-06-12 at 08:42 +0200, Jesper Dangaard Brouer wrote:
> > On Thu, 11 Jun 2020 20:36:40 -0400
> > Gaurav Singh <gaurav1086@gmail.com> wrote:
> >   
> > > Replace malloc/memset with calloc
> > > 
> > > Fixes: 0fca931a6f21 ("samples/bpf: program demonstrating access to xdp_rxq_info")
> > > Signed-off-by: Gaurav Singh <gaurav1086@gmail.com>  
> > 
> > Above is the correct use of Fixes + Signed-off-by.
> > 
> > Now you need to update/improve the description, to also
> > mention/describe that this also solves the bug you found.  
> 
> This is not a fix, it's a conversion of one
> correct code to a shorter one.

Read the code again Joe.  There is a bug in the code that gets removed,
as it runs memset on the memory before checking if it was NULL.

IHMO this proves why is it is necessary to mention in the commit
message, as you didn't notice the bug in your code review.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] xdp_rxq_info_user: Replace malloc/memset w/calloc
  2020-06-12 10:14     ` Joe Perches
  2020-06-12 12:05       ` Jesper Dangaard Brouer
@ 2020-06-12 12:06       ` Toke Høiland-Jørgensen
  1 sibling, 0 replies; 13+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-06-12 12:06 UTC (permalink / raw)
  To: Joe Perches, Jesper Dangaard Brouer, Gaurav Singh
  Cc: Alexei Starovoitov, Daniel Borkmann, David S. Miller,
	Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend,
	Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
	KP Singh, open list:XDP (eXpress Data Path),
	open list:XDP (eXpress Data Path),
	open list

Joe Perches <joe@perches.com> writes:

> On Fri, 2020-06-12 at 08:42 +0200, Jesper Dangaard Brouer wrote:
>> On Thu, 11 Jun 2020 20:36:40 -0400
>> Gaurav Singh <gaurav1086@gmail.com> wrote:
>> 
>> > Replace malloc/memset with calloc
>> > 
>> > Fixes: 0fca931a6f21 ("samples/bpf: program demonstrating access to xdp_rxq_info")
>> > Signed-off-by: Gaurav Singh <gaurav1086@gmail.com>
>> 
>> Above is the correct use of Fixes + Signed-off-by.
>> 
>> Now you need to update/improve the description, to also
>> mention/describe that this also solves the bug you found.
>
> This is not a fix, it's a conversion of one
> correct code to a shorter one.

No it isn't - the original code memset()s before it checks the return
from malloc(), so it's a potential NULL-pointer reference... Which the
commit message should explain, obviously :)

-Toke


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] xdp_rxq_info_user: Replace malloc/memset w/calloc
  2020-06-12 12:05       ` Jesper Dangaard Brouer
@ 2020-06-12 15:19         ` Joe Perches
  0 siblings, 0 replies; 13+ messages in thread
From: Joe Perches @ 2020-06-12 15:19 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: Gaurav Singh, Alexei Starovoitov, Daniel Borkmann,
	David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
	John Fastabend, Martin KaFai Lau, Song Liu, Yonghong Song,
	Andrii Nakryiko, KP Singh, open list:XDP (eXpress Data Path),
	open list:XDP (eXpress Data Path),
	open list

On Fri, 2020-06-12 at 14:05 +0200, Jesper Dangaard Brouer wrote:
> On Fri, 12 Jun 2020 03:14:58 -0700
> Joe Perches <joe@perches.com> wrote:
> 
> > On Fri, 2020-06-12 at 08:42 +0200, Jesper Dangaard Brouer wrote:
> > > On Thu, 11 Jun 2020 20:36:40 -0400
> > > Gaurav Singh <gaurav1086@gmail.com> wrote:
> > >   
> > > > Replace malloc/memset with calloc
> > > > 
> > > > Fixes: 0fca931a6f21 ("samples/bpf: program demonstrating access to xdp_rxq_info")
> > > > Signed-off-by: Gaurav Singh <gaurav1086@gmail.com>  
> > > 
> > > Above is the correct use of Fixes + Signed-off-by.
> > > 
> > > Now you need to update/improve the description, to also
> > > mention/describe that this also solves the bug you found.  
> > 
> > This is not a fix, it's a conversion of one
> > correct code to a shorter one.
> 
> Read the code again Joe.  There is a bug in the code that gets removed,
> as it runs memset on the memory before checking if it was NULL.
> 
> IHMO this proves why is it is necessary to mention in the commit
> message, as you didn't notice the bug in your code review.

I didn't review the code at all, just the commit message,

It's important to have commit messages that describe the
defect being corrected too.

Otherwise, a simple malloc/memset(0) vs zalloc equivalent
is not actually a defect.



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH] xdp_rxq_info_user: Fix null pointer dereference. Replace malloc/memset with calloc.
  2020-06-11 15:02 [PATCH] xdp_rxq_info_user: Replace malloc/memset w/calloc Gaurav Singh
                   ` (2 preceding siblings ...)
  2020-06-12  0:36 ` Gaurav Singh
@ 2020-06-12 18:53 ` Gaurav Singh
  2020-06-12 20:20   ` Jesper Dangaard Brouer
  2020-06-18  0:11 ` [PATCH] ia64: Add null pointer check for task in default_handler Gaurav Singh
  4 siblings, 1 reply; 13+ messages in thread
From: Gaurav Singh @ 2020-06-12 18:53 UTC (permalink / raw)
  To: gaurav1086, Alexei Starovoitov, Daniel Borkmann, David S. Miller,
	Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend,
	Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
	KP Singh, open list:XDP (eXpress Data Path),
	open list:XDP (eXpress Data Path),
	open list

Memset on the pointer right after malloc can cause a
null pointer deference if it failed to allocate memory.
A simple fix is to replace malloc/memset with a calloc()

Fixes: 0fca931a6f21 ("samples/bpf: program demonstrating access to xdp_rxq_info")
Signed-off-by: Gaurav Singh <gaurav1086@gmail.com>
---
 samples/bpf/xdp_rxq_info_user.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c
index 4fe47502ebed..caa4e7ffcfc7 100644
--- a/samples/bpf/xdp_rxq_info_user.c
+++ b/samples/bpf/xdp_rxq_info_user.c
@@ -198,11 +198,8 @@ static struct datarec *alloc_record_per_cpu(void)
 {
 	unsigned int nr_cpus = bpf_num_possible_cpus();
 	struct datarec *array;
-	size_t size;
 
-	size = sizeof(struct datarec) * nr_cpus;
-	array = malloc(size);
-	memset(array, 0, size);
+	array = calloc(nr_cpus, sizeof(struct datarec));
 	if (!array) {
 		fprintf(stderr, "Mem alloc error (nr_cpus:%u)\n", nr_cpus);
 		exit(EXIT_FAIL_MEM);
@@ -214,11 +211,8 @@ static struct record *alloc_record_per_rxq(void)
 {
 	unsigned int nr_rxqs = bpf_map__def(rx_queue_index_map)->max_entries;
 	struct record *array;
-	size_t size;
 
-	size = sizeof(struct record) * nr_rxqs;
-	array = malloc(size);
-	memset(array, 0, size);
+	array = calloc(nr_rxqs, sizeof(struct record));
 	if (!array) {
 		fprintf(stderr, "Mem alloc error (nr_rxqs:%u)\n", nr_rxqs);
 		exit(EXIT_FAIL_MEM);
@@ -232,8 +226,7 @@ static struct stats_record *alloc_stats_record(void)
 	struct stats_record *rec;
 	int i;
 
-	rec = malloc(sizeof(*rec));
-	memset(rec, 0, sizeof(*rec));
+	rec = calloc(1, sizeof(struct stats_record));
 	if (!rec) {
 		fprintf(stderr, "Mem alloc error\n");
 		exit(EXIT_FAIL_MEM);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] xdp_rxq_info_user: Fix null pointer dereference. Replace malloc/memset with calloc.
  2020-06-12 18:53 ` [PATCH] xdp_rxq_info_user: Fix null pointer dereference. Replace malloc/memset with calloc Gaurav Singh
@ 2020-06-12 20:20   ` Jesper Dangaard Brouer
  2020-06-12 22:58     ` John Fastabend
  0 siblings, 1 reply; 13+ messages in thread
From: Jesper Dangaard Brouer @ 2020-06-12 20:20 UTC (permalink / raw)
  To: Gaurav Singh
  Cc: brouer, Alexei Starovoitov, Daniel Borkmann, David S. Miller,
	Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend,
	Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
	KP Singh, open list:XDP (eXpress Data Path),
	open list:XDP (eXpress Data Path),
	open list

On Fri, 12 Jun 2020 14:53:27 -0400
Gaurav Singh <gaurav1086@gmail.com> wrote:

> Memset on the pointer right after malloc can cause a
> null pointer deference if it failed to allocate memory.
> A simple fix is to replace malloc/memset with a calloc()
> 
> Fixes: 0fca931a6f21 ("samples/bpf: program demonstrating access to xdp_rxq_info")
> Signed-off-by: Gaurav Singh <gaurav1086@gmail.com>

Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] xdp_rxq_info_user: Fix null pointer dereference. Replace malloc/memset with calloc.
  2020-06-12 20:20   ` Jesper Dangaard Brouer
@ 2020-06-12 22:58     ` John Fastabend
  0 siblings, 0 replies; 13+ messages in thread
From: John Fastabend @ 2020-06-12 22:58 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, Gaurav Singh
  Cc: brouer, Alexei Starovoitov, Daniel Borkmann, David S. Miller,
	Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend,
	Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
	KP Singh, (open list:XDP \(eXpress Data Path\)),
	open list:XDP (eXpress Data Path),
	(open list:XDP \(eXpress Data Path\) open list)

Jesper Dangaard Brouer wrote:
> On Fri, 12 Jun 2020 14:53:27 -0400
> Gaurav Singh <gaurav1086@gmail.com> wrote:
> 
> > Memset on the pointer right after malloc can cause a
> > null pointer deference if it failed to allocate memory.
> > A simple fix is to replace malloc/memset with a calloc()
> > 
> > Fixes: 0fca931a6f21 ("samples/bpf: program demonstrating access to xdp_rxq_info")
> > Signed-off-by: Gaurav Singh <gaurav1086@gmail.com>
> 
> Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
> 

Acked-by: John Fastabend <john.fastabend@gmail.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH] ia64: Add null pointer check for task in default_handler
  2020-06-11 15:02 [PATCH] xdp_rxq_info_user: Replace malloc/memset w/calloc Gaurav Singh
                   ` (3 preceding siblings ...)
  2020-06-12 18:53 ` [PATCH] xdp_rxq_info_user: Fix null pointer dereference. Replace malloc/memset with calloc Gaurav Singh
@ 2020-06-18  0:11 ` Gaurav Singh
  4 siblings, 0 replies; 13+ messages in thread
From: Gaurav Singh @ 2020-06-18  0:11 UTC (permalink / raw)
  To: gaurav1086, Tony Luck, Fenghua Yu,
	open list:IA64 (Itanium) PLATFORM, open list

If the task is NULL and the if condition is true 
then task->pid would cause null pointer dereference. 
Fix this by adding additional null check.

Signed-off-by: Gaurav Singh <gaurav1086@gmail.com>
---
 arch/ia64/kernel/perfmon_default_smpl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/ia64/kernel/perfmon_default_smpl.c b/arch/ia64/kernel/perfmon_default_smpl.c
index a40c56020fc5..2732b4cf4296 100644
--- a/arch/ia64/kernel/perfmon_default_smpl.c
+++ b/arch/ia64/kernel/perfmon_default_smpl.c
@@ -111,7 +111,7 @@ default_handler(struct task_struct *task, void *buf, pfm_ovfl_arg_t *arg, struct
 	unsigned char ovfl_notify;
 
 	if (unlikely(buf == NULL || arg == NULL|| regs == NULL || task == NULL)) {
-		DPRINT(("[%d] invalid arguments buf=%p arg=%p\n", task->pid, buf, arg));
+		DPRINT(("[%d] invalid arguments buf=%p arg=%p\n", task? task->pid : -1, buf, arg));
 		return -EINVAL;
 	}
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-06-18  0:12 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-11 15:02 [PATCH] xdp_rxq_info_user: Replace malloc/memset w/calloc Gaurav Singh
2020-06-11 18:01 ` Jesper Dangaard Brouer
2020-06-12  0:26 ` Gaurav Singh
2020-06-12  0:36 ` Gaurav Singh
2020-06-12  6:42   ` Jesper Dangaard Brouer
2020-06-12 10:14     ` Joe Perches
2020-06-12 12:05       ` Jesper Dangaard Brouer
2020-06-12 15:19         ` Joe Perches
2020-06-12 12:06       ` Toke Høiland-Jørgensen
2020-06-12 18:53 ` [PATCH] xdp_rxq_info_user: Fix null pointer dereference. Replace malloc/memset with calloc Gaurav Singh
2020-06-12 20:20   ` Jesper Dangaard Brouer
2020-06-12 22:58     ` John Fastabend
2020-06-18  0:11 ` [PATCH] ia64: Add null pointer check for task in default_handler Gaurav Singh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).