From: Nathan Rutman Date: Fri, 13 Aug 2010 17:06:24 +0000 (+0400) Subject: b=23298 backport some llapi_changelog/copytool fixes from HSM branch X-Git-Tag: 2.0.51.0~35 X-Git-Url: https://git.whamcloud.com/?p=fs%2Flustre-release.git;a=commitdiff_plain;h=d1edf5e2205468724a53d8138ae3f2b52297d10e b=23298 backport some llapi_changelog/copytool fixes from HSM branch 455f63ad2b92cea37793a3bce5d412b30ea144c5 zero kuc private memory d1949858dabc7de7738e2179cfc8e4b3d6fa799f make llapi_copytool_recv thread safe 9582d218eb6abef52814b364e16e5b3b24c29658 make llapi_changelog_recv thread safe --- diff --git a/libcfs/include/libcfs/libcfs_kernelcomm.h b/libcfs/include/libcfs/libcfs_kernelcomm.h index ff919b1..b2e5406 100644 --- a/libcfs/include/libcfs/libcfs_kernelcomm.h +++ b/libcfs/include/libcfs/libcfs_kernelcomm.h @@ -108,7 +108,7 @@ typedef struct lustre_kernelcomm { extern int libcfs_ukuc_start(lustre_kernelcomm *l, int groups); extern int libcfs_ukuc_stop(lustre_kernelcomm *l); extern int libcfs_ukuc_msg_get(lustre_kernelcomm *l, char *buf, int maxsize, - int transport); + int transport); #endif /* __LIBCFS_KERNELCOMM_H__ */ diff --git a/libcfs/libcfs/kernel_user_comm.c b/libcfs/libcfs/kernel_user_comm.c index a36a3bc..357bcf1 100644 --- a/libcfs/libcfs/kernel_user_comm.c +++ b/libcfs/libcfs/kernel_user_comm.c @@ -59,6 +59,7 @@ int libcfs_ukuc_start(lustre_kernelcomm *link, int group) if (pipe(pfd) < 0) return -errno; + memset(link, 0, sizeof(*link)); link->lk_rfd = pfd[0]; link->lk_wfd = pfd[1]; link->lk_group = group; @@ -79,7 +80,7 @@ int libcfs_ukuc_stop(lustre_kernelcomm *link) * Allocates memory, returns handle * * @param link Private descriptor for pipe/socket. - * @param buf Buffer to read into + * @param buf Buffer to read into, must include size for kuc_hdr * @param maxsize Maximum message size allowed * @param transport Only listen to messages on this transport * (and the generic transport) @@ -167,7 +168,7 @@ int libcfs_kkuc_msg_put(cfs_file_t *filp, void *payload) #ifdef __KERNEL__ { loff_t offset = 0; - rc = cfs_user_write(filp, (char *)payload, kuch->kuc_msglen, + rc = cfs_user_write(filp, (char *)payload, kuch->kuc_msglen, &offset); } #endif diff --git a/lustre/utils/lfs.c b/lustre/utils/lfs.c index 7754ccd..399fa13 100644 --- a/lustre/utils/lfs.c +++ b/lustre/utils/lfs.c @@ -2387,10 +2387,14 @@ static int lfs_changelog(int argc, char **argv) time_t secs; struct tm ts; - if (endrec && rec->cr_index > endrec) + if (endrec && rec->cr_index > endrec) { + llapi_changelog_free(&rec); break; - if (rec->cr_index < startrec) + } + if (rec->cr_index < startrec) { + llapi_changelog_free(&rec); continue; + } secs = rec->cr_time >> 30; gmtime_r(&secs, &ts); diff --git a/lustre/utils/liblustreapi.c b/lustre/utils/liblustreapi.c index a4c8cf6..d5292f7 100644 --- a/lustre/utils/liblustreapi.c +++ b/lustre/utils/liblustreapi.c @@ -2830,7 +2830,6 @@ struct changelog_private { int magic; int flags; lustre_kernelcomm kuc; - char *buf; }; /** Start reading from a changelog @@ -2847,16 +2846,10 @@ int llapi_changelog_start(void **priv, int flags, const char *device, int rc; /* Set up the receiver control struct */ - cp = malloc(sizeof(*cp)); + cp = calloc(1, sizeof(*cp)); if (cp == NULL) return -ENOMEM; - cp->buf = malloc(CR_MAXSIZE); - if (cp->buf == NULL) { - rc = -ENOMEM; - goto out_free; - } - cp->magic = CHANGELOG_PRIV_MAGIC; cp->flags = flags; @@ -2882,8 +2875,6 @@ int llapi_changelog_start(void **priv, int flags, const char *device, return 0; out_free: - if (cp->buf) - free(cp->buf); free(cp); return rc; } @@ -2897,7 +2888,6 @@ int llapi_changelog_fini(void **priv) return -EINVAL; libcfs_ukuc_stop(&cp->kuc); - free(cp->buf); free(cp); *priv = NULL; return 0; @@ -2920,14 +2910,17 @@ int llapi_changelog_recv(void *priv, struct changelog_rec **rech) return -EINVAL; if (rech == NULL) return -EINVAL; + kuch = malloc(CR_MAXSIZE + sizeof(*kuch)); + if (kuch == NULL) + return -ENOMEM; repeat: - rc = libcfs_ukuc_msg_get(&cp->kuc, cp->buf, CR_MAXSIZE, + rc = libcfs_ukuc_msg_get(&cp->kuc, (char *)kuch, + CR_MAXSIZE + sizeof(*kuch), KUC_TRANSPORT_CHANGELOG); if (rc < 0) - return rc; + goto out_free; - kuch = (struct kuc_hdr *)cp->buf; if ((kuch->kuc_transport != KUC_TRANSPORT_CHANGELOG) || ((kuch->kuc_msgtype != CL_RECORD) && (kuch->kuc_msgtype != CL_EOF))) { @@ -2948,19 +2941,30 @@ repeat: } } - /* Our message is a changelog_rec */ + /* Our message is a changelog_rec. Use pointer math to skip + * kuch_hdr and point directly to the message payload. + */ *rech = (struct changelog_rec *)(kuch + 1); return 0; out_free: *rech = NULL; + free(kuch); return rc; } /** Release the changelog record when done with it. */ int llapi_changelog_free(struct changelog_rec **rech) { + if (*rech) { + /* We allocated memory starting at the kuc_hdr, but passed + * the consumer a pointer to the payload. + * Use pointer math to get back to the header. + */ + struct kuc_hdr *kuch = (struct kuc_hdr *)*rech - 1; + free(kuch); + } *rech = NULL; return 0; } @@ -3078,7 +3082,6 @@ int llapi_path2fid(const char *path, lustre_fid *fid) #define CT_PRIV_MAGIC 0xC0BE2001 struct copytool_private { int magic; - char *buf; char *fsname; lustre_kernelcomm kuc; __u32 archives; @@ -3105,13 +3108,12 @@ int llapi_copytool_start(void **priv, char *fsname, int flags, return -EINVAL; } - ct = malloc(sizeof(*ct)); + ct = calloc(1, sizeof(*ct)); if (ct == NULL) return -ENOMEM; - ct->buf = malloc(HAL_MAXSIZE); ct->fsname = malloc(strlen(fsname) + 1); - if (ct->buf == NULL || ct->fsname == NULL) { + if (ct->fsname == NULL) { rc = -ENOMEM; goto out_err; } @@ -3149,8 +3151,6 @@ int llapi_copytool_start(void **priv, char *fsname, int flags, return 0; out_err: - if (ct->buf) - free(ct->buf); if (ct->fsname) free(ct->fsname); free(ct); @@ -3172,7 +3172,6 @@ int llapi_copytool_fini(void **priv) /* Shut down the kernelcomms */ libcfs_ukuc_stop(&ct->kuc); - free(ct->buf); free(ct->fsname); free(ct); *priv = NULL; @@ -3198,13 +3197,17 @@ int llapi_copytool_recv(void *priv, struct hsm_action_list **halh, int *msgsize) if (halh == NULL || msgsize == NULL) return -EINVAL; - rc = libcfs_ukuc_msg_get(&ct->kuc, ct->buf, HAL_MAXSIZE, + kuch = malloc(HAL_MAXSIZE + sizeof(*kuch)); + if (kuch == NULL) + return -ENOMEM; + + rc = libcfs_ukuc_msg_get(&ct->kuc, (char *)kuch, + HAL_MAXSIZE + sizeof(*kuch), KUC_TRANSPORT_HSM); if (rc < 0) - return rc; + goto out_free; /* Handle generic messages */ - kuch = (struct kuc_hdr *)ct->buf; if (kuch->kuc_transport == KUC_TRANSPORT_GENERIC && kuch->kuc_msgtype == KUC_MSG_SHUTDOWN) { rc = -ESHUTDOWN; @@ -3220,8 +3223,9 @@ int llapi_copytool_recv(void *priv, struct hsm_action_list **halh, int *msgsize) goto out_free; } - /* Our message is an hsm_action_list */ - + /* Our message is a hsm_action_list. Use pointer math to skip + * kuch_hdr and point directly to the message payload. + */ hal = (struct hsm_action_list *)(kuch + 1); /* Check that we have registered for this archive # */ @@ -3240,14 +3244,15 @@ int llapi_copytool_recv(void *priv, struct hsm_action_list **halh, int *msgsize) out_free: *halh = NULL; *msgsize = 0; + free(kuch); return rc; } /** Release the action list when done with it. */ int llapi_copytool_free(struct hsm_action_list **hal) { - *hal = NULL; - return 0; + /* Reuse the llapi_changelog_free function */ + return llapi_changelog_free((struct changelog_rec **)hal); } int llapi_get_connect_flags(const char *mnt, __u64 *flags)