[PATCH 0/3] rasdaemon: misc fixes

[PATCH 0/3] rasdaemon: misc fixes

[arozansk]

These patches are for 3 issues found with covscan. None of them are likely to
happen during regular use, but worth fixing anyway.

Signed-off-by: Aristeu Rozanski <arozansk@redhat.com>

-- 
Aristeu

[PATCH 1/3] rasdaemon: ras-report: fix possible but unlikely file descriptor leak

[arozansk]

Found with covscan.

Signed-off-by: Aristeu Rozanski <arozansk@redhat.com>

diff --git a/ras-report.c b/ras-report.c
index ea3a9b6..0088745 100644
--- a/ras-report.c
+++ b/ras-report.c
@@ -768,7 +768,7 @@ int ras_report_mf_event(struct ras_events *ras, struct ras_mf_event *ev)
 	done = 1;
 
 mf_fail:
-	if (sockfd > 0)
+	if (sockfd >= 0)
 		close(sockfd);
 
 	if (done)

[PATCH 2/3] rasdaemon: mce-amd-smca: properly limit bank types

[arozansk]

Found with covscan.

Signed-off-by: Aristeu Rozanski <arozansk@redhat.com>

diff --git a/mce-amd-smca.c b/mce-amd-smca.c
index f3379fc..27ca8aa 100644
--- a/mce-amd-smca.c
+++ b/mce-amd-smca.c
@@ -584,7 +584,7 @@ static void decode_smca_error(struct mce_event *e)
 		return;
 	}
 
-	if (bank_type >= MAX_NR_BANKS) {
+	if (bank_type >= N_SMCA_BANK_TYPES) {
 		strcpy(e->mcastatus_msg, "Don't know how to decode this bank");
 		return;
 	}

[PATCH 3/3] rasdaemon: ras-memory-failure-handler: handle localtime() failure correctly

[arozansk]

We could just have an empty string but keeping the format could prevent
issues if someone is actually parsing this.
Found with covscan.

Signed-off-by: Aristeu Rozanski <arozansk@redhat.com>

diff --git a/ras-memory-failure-handler.c b/ras-memory-failure-handler.c
index 9941e68..9574141 100644
--- a/ras-memory-failure-handler.c
+++ b/ras-memory-failure-handler.c
@@ -148,6 +148,8 @@ int ras_memory_failure_event_handler(struct trace_seq *s,
 	if (tm)
 		strftime(ev.timestamp, sizeof(ev.timestamp),
 			 "%Y-%m-%d %H:%M:%S %z", tm);
+	else
+		strncpy(ev.timestamp, "0000-00-00 00:00:00 GMT", sizeof(ev.timestamp));
 	trace_seq_printf(s, "%s ", ev.timestamp);
 
 	if (pevent_get_field_val(s,  event, "pfn", record, &val, 1) < 0)

RE: [PATCH 3/3] rasdaemon: ras-memory-failure-handler: handle localtime() failure correctly

[Elliott, Robert (Servers)]

> -----Original Message-----
> From: arozansk@redhat.com <arozansk@redhat.com>
> Sent: Wednesday, July 20, 2022 9:40 AM
...
> Subject: [PATCH 3/3] rasdaemon: ras-memory-failure-handler: handle
> localtime() failure correctly
> 
> We could just have an empty string but keeping the format could prevent
> issues if someone is actually parsing this.
> Found with covscan.
> 
...
> diff --git a/ras-memory-failure-handler.c b/ras-memory-failure-handler.c
...
> @@ -148,6 +148,8 @@ int ras_memory_failure_event_handler(struct trace_seq
> *s,
>  	if (tm)
>  		strftime(ev.timestamp, sizeof(ev.timestamp),
>  			 "%Y-%m-%d %H:%M:%S %z", tm);
> +	else
> +		strncpy(ev.timestamp, "0000-00-00 00:00:00 GMT", sizeof(ev.timestamp));


Per man strftime:
       %z     The +hhmm or -hhmm numeric timezone (that is, the hour and minute offset from UTC). (SU)
       %Z     The timezone name or abbreviation.

GMT does not match the lowercase %z format of +hhmm or -hhmm

Returning the UNIX epoch might be safer, too:
    1970-01-01 00:00:00 +0000

Re: [PATCH 3/3] rasdaemon: ras-memory-failure-handler: handle localtime() failure correctly

['arozansk@redhat.com']

On Wed, Jul 20, 2022 at 03:33:49PM +0000, Elliott, Robert (Servers) wrote:
> > -----Original Message-----
> > From: arozansk@redhat.com <arozansk@redhat.com>
> > Sent: Wednesday, July 20, 2022 9:40 AM
> ...
> > Subject: [PATCH 3/3] rasdaemon: ras-memory-failure-handler: handle
> > localtime() failure correctly
> > 
> > We could just have an empty string but keeping the format could prevent
> > issues if someone is actually parsing this.
> > Found with covscan.
> > 
> ...
> > diff --git a/ras-memory-failure-handler.c b/ras-memory-failure-handler.c
> ...
> > @@ -148,6 +148,8 @@ int ras_memory_failure_event_handler(struct trace_seq
> > *s,
> >  	if (tm)
> >  		strftime(ev.timestamp, sizeof(ev.timestamp),
> >  			 "%Y-%m-%d %H:%M:%S %z", tm);
> > +	else
> > +		strncpy(ev.timestamp, "0000-00-00 00:00:00 GMT", sizeof(ev.timestamp));
> 
> 
> Per man strftime:
>        %z     The +hhmm or -hhmm numeric timezone (that is, the hour and minute offset from UTC). (SU)
>        %Z     The timezone name or abbreviation.
> 
> GMT does not match the lowercase %z format of +hhmm or -hhmm

Good catch, thanks

> Returning the UNIX epoch might be safer, too:
>     1970-01-01 00:00:00 +0000

Agreed, v2 will follow

-- 
Aristeu

[PATCH 3/3 v2] rasdaemon: ras-memory-failure-handler: handle localtime() failure correctly

[arozansk]

We could just have an empty string but keeping the format could prevent
issues if someone is actually parsing this.
Found with covscan.

v2: fixed the timestamp as pointed by Robert Elliott

Signed-off-by: Aristeu Rozanski <arozansk@redhat.com>

diff --git a/ras-memory-failure-handler.c b/ras-memory-failure-handler.c
index 9941e68..9574141 100644
--- a/ras-memory-failure-handler.c
+++ b/ras-memory-failure-handler.c
@@ -148,6 +148,8 @@ int ras_memory_failure_event_handler(struct trace_seq *s,
 	if (tm)
 		strftime(ev.timestamp, sizeof(ev.timestamp),
 			 "%Y-%m-%d %H:%M:%S %z", tm);
+	else
+		strncpy(ev.timestamp, "1970-01-01 00:00:00 +0000", sizeof(ev.timestamp));
 	trace_seq_printf(s, "%s ", ev.timestamp);
 
 	if (pevent_get_field_val(s,  event, "pfn", record, &val, 1) < 0)