From 727638a1d0e72a043c2798f081f166a0e3a39268 Mon Sep 17 00:00:00 2001 From: Andreas Dilger Date: Thu, 8 Dec 2022 11:43:57 -0700 Subject: [PATCH] LU-16376 obdclass: NUL terminate long jobid strings It appears that some jobid names can be sent that are using the full 32-byte size, rather than containing an embedded NUL terminator. This caused errors in lprocfs_job_stats_log() when it overflowed. If there is no NUL terminator in lustre_msg_get_jobid() then add one if not found within the buffer, so that the rest of the code doesn't have to deal with unterminated strings. This potentially exposes a larger issue that other places may not be handling the unterminated string properly either, which needs to be addressed separately on both the client and server. Terminating the jobid to 31 chars only on the client does not totally solve the issue, since there will still be older clients that are not doing this, so the server needs to handle this in any case. Lustre-change: https://review.whamcloud.com/49351 Lustre-commit: 9eba5d57297f807fddf046356c846478bbf232f4 Signed-off-by: Andreas Dilger Change-Id: I4c05fabdacb6a0bbf6477d3601a628fe1f3ebbe5 Reviewed-by: Feng Lei Reviewed-by: James Simmons Reviewed-by: Neil Brown Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/49490 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Oleg Drokin --- lustre/obdclass/lprocfs_jobstats.c | 5 +++-- lustre/ptlrpc/pack_generic.c | 6 ++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/lustre/obdclass/lprocfs_jobstats.c b/lustre/obdclass/lprocfs_jobstats.c index 6952957..4202f44 100644 --- a/lustre/obdclass/lprocfs_jobstats.c +++ b/lustre/obdclass/lprocfs_jobstats.c @@ -286,10 +286,11 @@ int lprocfs_job_stats_log(struct obd_device *obd, char *jobid, RETURN(-EINVAL); if (jobid == NULL || strlen(jobid) == 0) - RETURN(-EINVAL); + RETURN(0); + /* unterminated jobid should be handled in lustre_msg_get_jobid() */ if (strlen(jobid) >= LUSTRE_JOBID_SIZE) { - CERROR("Invalid jobid size (%lu), expect(%d)\n", + CERROR("%s: invalid jobid size %lu, expect %d\n", obd->obd_name, (unsigned long)strlen(jobid) + 1, LUSTRE_JOBID_SIZE); RETURN(-EINVAL); } diff --git a/lustre/ptlrpc/pack_generic.c b/lustre/ptlrpc/pack_generic.c index e3c0f47..3263e94 100644 --- a/lustre/ptlrpc/pack_generic.c +++ b/lustre/ptlrpc/pack_generic.c @@ -1324,6 +1324,12 @@ char *lustre_msg_get_jobid(struct lustre_msg *msg) if (!pb) return NULL; + /* If clients send unterminated jobids, terminate them here + * so that there is no chance of string overflow later. + */ + if (unlikely(pb->pb_jobid[LUSTRE_JOBID_SIZE - 1] != '\0')) + pb->pb_jobid[LUSTRE_JOBID_SIZE - 1] = '\0'; + return pb->pb_jobid; } default: -- 1.8.3.1