source buffer is treated more as a filehandle than a string.
* amd/nfs_subr.c (fh_to_mp3): Replace xstrlcpy with memcpy because the
source buffer is treated more as a filehandle than a string.
* amd/opts.c (free_op): No longer need to assign pointer to NULL
after XFREE.
* amd/opts.c (expand_op): Revert back to using strncpy() instead
of xstrlcpy. The code is correct and relies on the semantics of
strncpy.
* libamu/mount_fs.c (compute_nfs_args): Leave XXX warning that use
of xstrlcpy in NFS_HN_DREF may corrupt a struct nfs_args, or
truncate our concocted "hostname:/path" string prematurely if the
nap->hostname field is ever less than 64 bytes long
(MAXHOSTNAMELEN).
* libamu/util.c (xstrlcpy): Return immediately if len is 0 to
avoid unnecessary work. Log an error and return if len is less
than 0.
+2005-04-09 Daniel P. Ottavio <dottavio@ic.sunysb.edu>
+
+ * amd/nfs_subr.c (mp_to_fh): Replace xstrlcpy with memcpy because the
+ source buffer is treated more as a filehandle than a string.
+
+ * amd/nfs_subr.c (fh_to_mp3): Replace xstrlcpy with memcpy because the
+ source buffer is treated more as a filehandle than a string.
+
+ * amd/opts.c (free_op): No longer need to assign pointer to NULL
+ after XFREE.
+
+ * amd/opts.c (expand_op): Revert back to using strncpy() instead
+ of xstrlcpy. The code is correct and relies on the semantics of
+ strncpy.
+
+ * libamu/mount_fs.c (compute_nfs_args): Leave XXX warning that use
+ of xstrlcpy in NFS_HN_DREF may corrupt a struct nfs_args, or
+ truncate our concocted "hostname:/path" string prematurely if the
+ nap->hostname field is ever less than 64 bytes long
+ (MAXHOSTNAMELEN).
+
+ * libamu/util.c (xstrlcpy): Return immediately if len is 0 to
+ avoid unnecessary work. Log an error and return if len is less
+ than 0.
+
2005-04-07 Erez Zadok <ezk@cs.sunysb.edu>
* include/am_utils.h (XFREE): XFREE() should nullify the pointer
*** Notes specific to am-utils version 6.1-rc2
-Using a custom version of strlcpy instead of strncpy, to minimize string
-overlow changes. Code is safer now.
+Using a custom version of strlcpy instead of strncpy (but only where
+it makes sense), to minimize string overlow changes. Audited all use
+of strncpy/strlcpy to ensure safety.
- bugs fixed:
* pawd handles all file systems
* SUCH DAMAGE.
*
*
- * $Id: nfs_subr.c,v 1.27 2005/04/07 05:50:38 ezk Exp $
+ * $Id: nfs_subr.c,v 1.28 2005/04/09 18:15:35 ottavio Exp $
*
*/
if (fp->fhh_type != 0) {
/* New filehandle type */
- char *path = xmalloc(sizeof(*fhp));
- xstrlcpy(path, (char *) fhp, sizeof(*fhp));
+ int len = sizeof(*fhp);
+ char *path = xmalloc(len+1);
+ /*
+ * Because fhp is treated as a filehandle we use memcpy
+ * instead of xstrlcpy.
+ */
+ memcpy(path, (char *) fhp, len);
+ path[len] = '\0';
/* dlog("fh_to_mp3: new filehandle: %s", path); */
ap = path_to_exported_ap(path);
pathlen = strlen(mp->am_path);
if (pathlen <= sizeof(*fhp)) {
/* dlog("mp_to_fh: new filehandle: %s", mp->am_path); */
- xstrlcpy((char *) fhp, mp->am_path, pathlen);
+ /*
+ * Because fhp is treated as a filehandle we use memcpy instead of
+ * xstrlcpy.
+ */
+ memcpy((char *) fhp, mp->am_path, pathlen); /* making a filehandle */
} else {
struct am_fh *fp = (struct am_fh *) fhp;
* SUCH DAMAGE.
*
*
- * $Id: opts.c,v 1.37 2005/04/07 05:50:38 ezk Exp $
+ * $Id: opts.c,v 1.38 2005/04/09 18:15:35 ottavio Exp $
*
*/
{
if (*p->opt) {
XFREE(*p->opt);
- *p->opt = 0;
}
}
*/
{
int len = dp - cp;
-
- if (BUFSPACE(ep, len)) {
- xstrlcpy(ep, cp, len);
- ep += len;
- } else {
- plog(XLOG_ERROR, EXPAND_ERROR, opt);
- goto out;
+
+ if (len > 0) {
+ if (BUFSPACE(ep, len)) {
+ /*
+ * We use strncpy (not xstrlen) because 'ep' relies on it's symantics.
+ * BUFSPACE guarantees that ep can hold len.
+ */
+ strncpy(ep, cp, len);
+ ep += len;
+ } else {
+ plog(XLOG_ERROR, EXPAND_ERROR, opt);
+ goto out;
+ }
}
}
/*
* Put the string into another buffer so
* we can do comparisons.
+ *
+ * We use strncpy here (not xstrlcpy) because the dest is meant
+ * to be truncated and we don't want to log it as an error. The
+ * use of the BUFSPACE macro above guarantees the safe use of
+ * strncpy with nbuf.
*/
- xstrlcpy(nbuf, cp, len);
-
+ strncpy(nbuf, cp, len);
+ nbuf[len] = '\0';
+
/*
* Advance cp
*/
* SUCH DAMAGE.
*
*
- * $Id: mount_fs.c,v 1.49 2005/03/21 00:16:53 ezk Exp $
+ * $Id: mount_fs.c,v 1.50 2005/04/09 18:15:35 ottavio Exp $
*
*/
/************************************************************************/
/*** HOST NAME ***/
/************************************************************************/
+ /*
+ * XXX: warning, using xstrlcpy in NFS_HN_DREF, which may corrupt a
+ * struct nfs_args, or truncate our concocted "hostname:/path"
+ * string prematurely.
+ */
NFS_HN_DREF(nap->hostname, host_name);
#ifdef MNT2_NFS_OPT_HOSTNAME
nap->flags |= MNT2_NFS_OPT_HOSTNAME;
* SUCH DAMAGE.
*
*
- * $Id: strutil.c,v 1.15 2005/04/07 05:09:16 ezk Exp $
+ * $Id: strutil.c,v 1.16 2005/04/09 18:15:35 ottavio Exp $
*
*/
void
xstrlcpy(char *dst, const char *src, size_t len)
{
+ if (len < 0) {
+ plog(XLOG_ERROR, "xstrlcpy: illegal len %d", len);
+ return;
+ }
+ if (len == 0)
+ return;
if (strlcpy(dst, src, len) >= len)
plog(XLOG_ERROR, "xstrlcpy: string \"%s\" truncated to \"%s\"", src, dst);
}