[slurm-dev] strftime issue(s).

[Magnus Jonsson]

Hi!




Reproducable with (in 2.5.{6,7}):

OK:

2013-06-14T11:40:14$
2013-06-14T11:40:14$
2013-06-14T11:40:11$


Fail:

End$
2013-06-14TVM-^P^?$
2013-06-14TVM-^P^?$
2013-06-14TVM-^P^?$





From man page of strftime (Ubuntu Precise):

------8<------------
RETURN VALUE
The  strftime() function returns the number of characters placed in the
array s, not including the terminating null byte, provided the  string,
including  the  terminating  null byte, fits.  Otherwise, it returns 0,
and the contents of the array is  undefined.   (This  behavior  applies
since  at  least  libc  4.4.4;  very old versions of libc, such as libc
4.4.1, would return max if the array was too small.)

Note that the return value 0 does not necessarily  indicate  an  error;
for example, in many locales %p yields an empty string.
------8<------------

From man page of strftime (Solaris)
------8<------------

------8<------------



As I see it the problem can be solved in many ways.









Best regards,
Magnus

--
Magnus Jonsson, Developer, HPC2N, Umeå Universitet

diff -ru site/src/common/parse_time.c amd64_ubuntu1004/src/common/parse_time.c
--- site/src/common/parse_time.c	2013-06-05 21:43:00.000000000 +0200
+++ amd64_ubuntu1004/src/common/parse_time.c	2013-06-14 16:56:20.000000000 +0200
@@ -597,6 +597,7 @@
 		static char fmt_buf[32];
 		static const char *display_fmt;
 		static bool use_relative_format;
+		char tmp_string[(size<256?256:size+1)];
 
 		if (!display_fmt) {
 			char *fmt = getenv("SLURM_TIME_FORMAT");
@@ -626,7 +627,11 @@
 		if (use_relative_format)
 			display_fmt = _relative_date_fmt(&time_tm);
 
-		strftime(string, size, display_fmt, &time_tm);
+		if(strftime(tmp_string, sizeof(tmp_string), display_fmt, &time_tm) == 0) {
+			memset(tmp_string,'#',size);
+		}
+		strncpy(string,tmp_string,size);
+		string[size-1] = 0;
 	}
 }

We found an issue in sacct that we pined down to a strftime call in 'src/common/parse_time.c' (slurm_make_time_str). % sacct -Po end%19 -s failed,completed,timeout,cancelled -S 2013-06-13 | head | cat -vet sacct -Po end%18 -s failed,completed,timeout,cancelled -S 2013-06-13 | head | cat -vet The problem is that the output from strftime with a buffer less the the required length of output is undefined in libc since libc 4.4.4. Different libc implementations seams to implement this differently. On Solaris for example the buffer is truncated at the given length but still returns 0. If the total number of resulting characters including the terminating null character is more than maxsize, strftime() returns 0 and the contents of the array are indeterminate. The return value of strftime is not checked for in slurm_make_time_str making the returned value undefined. 1. using a "large" temporary buffer for the output the expected behaviour of sacct end%N will for most normal cases be fine. 2. Only checking the return value and returning an error or an well defined output. I haved attached a patch for case 1 with that sets the output to "#" if the output still not fit into the buffer. There seams to be other places in the slurm code base that uses strftime and not checking the return code. Some of them might be OK due to the format string and size of the buffer. But this might needs to be looked into with more depth.

smime.p7s
Description: S/MIME Cryptographic Signature