From b18cecc6a17e9b30064ed9b24927d1905729d89b Mon Sep 17 00:00:00 2001 From: Bobi Jam Date: Fri, 13 Jan 2012 13:46:07 +0800 Subject: [PATCH] LU-985 lprocfs: verify user buffer access In lprocfs_xxx_evict_client(), need verify user's buffer when access it. Signed-off-by: Bobi Jam Change-Id: I702e22f8d432edce200c6d91a0af8a1eac792008 Reviewed-on: http://review.whamcloud.com/1961 Tested-by: Hudson Tested-by: Maloo Reviewed-by: Liang Zhen Reviewed-by: Andreas Dilger Reviewed-by: Oleg Drokin --- libcfs/include/libcfs/libcfs_string.h | 3 ++ libcfs/libcfs/libcfs_string.c | 28 +++++++++++++++ lustre/mds/lproc_mds.c | 45 +++++++++++++++++------ lustre/mdt/mdt_lproc.c | 67 +++++++++++++++++++++++++++++------ lustre/ptlrpc/lproc_ptlrpc.c | 25 +++++++++++-- 5 files changed, 146 insertions(+), 22 deletions(-) diff --git a/libcfs/include/libcfs/libcfs_string.h b/libcfs/include/libcfs/libcfs_string.h index ab03578..49358212 100644 --- a/libcfs/include/libcfs/libcfs_string.h +++ b/libcfs/include/libcfs/libcfs_string.h @@ -58,4 +58,7 @@ int cfs_vsnprintf(char *buf, size_t size, const char *fmt, va_list args); /* safe snprintf */ int cfs_snprintf(char *buf, size_t size, const char *fmt, ...); + +/* trim leading and trailing space characters */ +char *cfs_firststr(char *str, size_t size); #endif diff --git a/libcfs/libcfs/libcfs_string.c b/libcfs/libcfs/libcfs_string.c index 83ad82b..4655bc0 100644 --- a/libcfs/libcfs/libcfs_string.c +++ b/libcfs/libcfs/libcfs_string.c @@ -182,3 +182,31 @@ int cfs_snprintf(char *buf, size_t size, const char *fmt, ...) return i; } EXPORT_SYMBOL(cfs_snprintf); + +/* get the first string out of @str */ +char *cfs_firststr(char *str, size_t size) +{ + size_t i = 0; + char *end; + + /* trim leading spaces */ + while (i < size && *str && isspace(*str)) { + ++i; + ++str; + } + + /* string with all spaces */ + if (*str == '\0') + goto out; + + end = str; + while (i < size && *end != '\0' && !isspace(*end)) { + ++i; + ++end; + } + + *end= '\0'; +out: + return str; +} +EXPORT_SYMBOL(cfs_firststr); diff --git a/lustre/mds/lproc_mds.c b/lustre/mds/lproc_mds.c index 343d3fe..ba7a4cc 100644 --- a/lustre/mds/lproc_mds.c +++ b/lustre/mds/lproc_mds.c @@ -68,23 +68,44 @@ static int lprocfs_mds_wr_evictostnids(struct file *file, const char *buffer, return count; } +#define BUFLEN (UUID_MAX + 4) + static int lprocfs_mds_wr_evict_client(struct file *file, const char *buffer, unsigned long count, void *data) { - struct obd_device *obd = data; - struct mds_obd *mds = &obd->u.mds; - char tmpbuf[sizeof(struct obd_uuid)]; struct ptlrpc_request_set *set; - int rc; + struct obd_device *obd = data; + struct mds_obd *mds = &obd->u.mds; + char *kbuf; + char *tmpbuf; + int rc; + + OBD_ALLOC(kbuf, BUFLEN); + if (kbuf == NULL) + return -ENOMEM; - sscanf(buffer, "%40s", tmpbuf); + /* + * OBD_ALLOC() will zero kbuf, but we only copy BUFLEN - 1 + * bytes into kbuf, to ensure that the string is NUL-terminated. + * UUID_MAX should include a trailing NUL already. + */ + if (cfs_copy_from_user(kbuf, buffer, + min_t(unsigned long, BUFLEN - 1, count))) { + count = -EFAULT; + goto out; + } + tmpbuf = cfs_firststr(kbuf, min_t(unsigned long, BUFLEN - 1, count)); - if (strncmp(tmpbuf, "nid:", 4) != 0) - return lprocfs_wr_evict_client(file, buffer, count, data); + if (strncmp(tmpbuf, "nid:", 4) != 0) { + count = lprocfs_wr_evict_client(file, buffer, count, data); + goto out; + } set = ptlrpc_prep_set(); - if (!set) - return -ENOMEM; + if (set == NULL) { + count = -ENOMEM; + goto out; + } if (obd->u.mds.mds_evict_ost_nids) { rc = obd_set_info_async(mds->mds_lov_exp, @@ -103,7 +124,7 @@ static int lprocfs_mds_wr_evict_client(struct file *file, const char *buffer, class_incref(obd, __FUNCTION__, cfs_current()); LPROCFS_EXIT(); - obd_export_evict_by_nid(obd, tmpbuf+4); + obd_export_evict_by_nid(obd, tmpbuf + 4); rc = ptlrpc_set_wait(set); @@ -115,9 +136,13 @@ static int lprocfs_mds_wr_evict_client(struct file *file, const char *buffer, class_decref(obd, __FUNCTION__, cfs_current()); ptlrpc_set_destroy(set); +out: + OBD_FREE(kbuf, BUFLEN); return count; } +#undef BUFLEN + static int lprocfs_wr_atime_diff(struct file *file, const char *buffer, unsigned long count, void *data) { diff --git a/lustre/mdt/mdt_lproc.c b/lustre/mdt/mdt_lproc.c index 3417cae..e3da0d9 100644 --- a/lustre/mdt/mdt_lproc.c +++ b/lustre/mdt/mdt_lproc.c @@ -619,21 +619,44 @@ static int lprocfs_wr_ck_timeout(struct file *file, const char *buffer, return count; } +#define BUFLEN (UUID_MAX + 4) + static int lprocfs_mdt_wr_evict_client(struct file *file, const char *buffer, unsigned long count, void *data) { - char tmpbuf[sizeof(struct obd_uuid)]; + char *kbuf; + char *tmpbuf; - sscanf(buffer, "%40s", tmpbuf); + OBD_ALLOC(kbuf, BUFLEN); + if (kbuf == NULL) + return -ENOMEM; - if (strncmp(tmpbuf, "nid:", 4) != 0) - return lprocfs_wr_evict_client(file, buffer, count, data); + /* + * OBD_ALLOC() will zero kbuf, but we only copy BUFLEN - 1 + * bytes into kbuf, to ensure that the string is NUL-terminated. + * UUID_MAX should include a trailing NUL already. + */ + if (cfs_copy_from_user(kbuf, buffer, + min_t(unsigned long, BUFLEN - 1, count))) { + count = -EFAULT; + goto out; + } + tmpbuf = cfs_firststr(kbuf, min_t(unsigned long, BUFLEN - 1, count)); + + if (strncmp(tmpbuf, "nid:", 4) != 0) { + count = lprocfs_wr_evict_client(file, buffer, count, data); + goto out; + } CERROR("NOT implement evict client by nid %s\n", tmpbuf); +out: + OBD_FREE(kbuf, BUFLEN); return count; } +#undef BUFLEN + static int lprocfs_rd_sec_level(char *page, char **start, off_t off, int count, int *eof, void *data) { @@ -916,21 +939,45 @@ static int lprocfs_mdt_wr_mdc(struct file *file, const char *buffer, { struct obd_device *obd = data; struct obd_export *exp = NULL; - struct obd_uuid uuid; - char tmpbuf[sizeof(struct obd_uuid)]; + struct obd_uuid *uuid; + char *kbuf; + char *tmpbuf; - sscanf(buffer, "%40s", tmpbuf); + OBD_ALLOC(kbuf, UUID_MAX); + if (kbuf == NULL) + return -ENOMEM; + + /* + * OBD_ALLOC() will zero kbuf, but we only copy UUID_MAX - 1 + * bytes into kbuf, to ensure that the string is NUL-terminated. + * UUID_MAX should include a trailing NUL already. + */ + if (cfs_copy_from_user(kbuf, buffer, + min_t(unsigned long, UUID_MAX - 1, count))) { + count = -EFAULT; + goto out; + } + tmpbuf = cfs_firststr(kbuf, min_t(unsigned long, UUID_MAX - 1, count)); - obd_str2uuid(&uuid, tmpbuf); - exp = cfs_hash_lookup(obd->obd_uuid_hash, &uuid); + OBD_ALLOC(uuid, UUID_MAX); + if (uuid == NULL) { + count = -ENOMEM; + goto out; + } + + obd_str2uuid(uuid, tmpbuf); + exp = cfs_hash_lookup(obd->obd_uuid_hash, uuid); if (exp == NULL) { CERROR("%s: no export %s found\n", - obd->obd_name, obd_uuid2str(&uuid)); + obd->obd_name, obd_uuid2str(uuid)); } else { mdt_hsm_copytool_send(exp); class_export_put(exp); } + OBD_FREE(uuid, UUID_MAX); +out: + OBD_FREE(kbuf, UUID_MAX); return count; } diff --git a/lustre/ptlrpc/lproc_ptlrpc.c b/lustre/ptlrpc/lproc_ptlrpc.c index 2b5cefb..4fd4403 100644 --- a/lustre/ptlrpc/lproc_ptlrpc.c +++ b/lustre/ptlrpc/lproc_ptlrpc.c @@ -744,12 +744,30 @@ void ptlrpc_lprocfs_unregister_obd(struct obd_device *obd) EXPORT_SYMBOL(ptlrpc_lprocfs_unregister_obd); +#define BUFLEN (UUID_MAX + 5) + int lprocfs_wr_evict_client(struct file *file, const char *buffer, unsigned long count, void *data) { struct obd_device *obd = data; - char tmpbuf[sizeof(struct obd_uuid)]; + char *kbuf; + char *tmpbuf; + + OBD_ALLOC(kbuf, BUFLEN); + if (kbuf == NULL) + return -ENOMEM; + /* + * OBD_ALLOC() will zero kbuf, but we only copy BUFLEN - 1 + * bytes into kbuf, to ensure that the string is NUL-terminated. + * UUID_MAX should include a trailing NUL already. + */ + if (cfs_copy_from_user(kbuf, buffer, + min_t(unsigned long, BUFLEN - 1, count))) { + count = -EFAULT; + goto out; + } + tmpbuf = cfs_firststr(kbuf, min_t(unsigned long, BUFLEN - 1, count)); /* Kludge code(deadlock situation): the lprocfs lock has been held * since the client is evicted by writting client's * uuid/nid to procfs "evict_client" entry. However, @@ -760,7 +778,6 @@ int lprocfs_wr_evict_client(struct file *file, const char *buffer, class_incref(obd, __FUNCTION__, cfs_current()); LPROCFS_EXIT(); - sscanf(buffer, "%40s", tmpbuf); if (strncmp(tmpbuf, "nid:", 4) == 0) obd_export_evict_by_nid(obd, tmpbuf + 4); else if (strncmp(tmpbuf, "uuid:", 5) == 0) @@ -771,10 +788,14 @@ int lprocfs_wr_evict_client(struct file *file, const char *buffer, LPROCFS_ENTRY(); class_decref(obd, __FUNCTION__, cfs_current()); +out: + OBD_FREE(kbuf, BUFLEN); return count; } EXPORT_SYMBOL(lprocfs_wr_evict_client); +#undef BUFLEN + int lprocfs_wr_ping(struct file *file, const char *buffer, unsigned long count, void *data) { -- 1.8.3.1