Commit cf0d7e7f authored by Kees Cook's avatar Kees Cook Committed by Anna Schumaker
Browse files

NFS: Avoid memcpy() run-time warning for struct sockaddr overflows 



The 'nfs_server' and 'mount_server' structures include a union of
'struct sockaddr' (with the older 16 bytes max address size) and
'struct sockaddr_storage' which is large enough to hold all the
supported sa_family types (128 bytes max size). The runtime memcpy()
buffer overflow checker is seeing attempts to write beyond the 16
bytes as an overflow, but the actual expected size is that of 'struct
sockaddr_storage'. Plumb the use of 'struct sockaddr_storage' more
completely through-out NFS, which results in adjusting the memcpy()
buffers to the correct union members. Avoids this false positive run-time
warning under CONFIG_FORTIFY_SOURCE:

  memcpy: detected field-spanning write (size 28) of single field "&ctx->nfs_server.address" at fs/nfs/namespace.c:178 (size 16)

Reported-by: default avatarkernel test robot <yujie.liu@intel.com>
Link: https://lore.kernel.org/all/202210110948.26b43120-yujie.liu@intel.com


Cc: Trond Myklebust <trond.myklebust@hammerspace.com>
Cc: Anna Schumaker <anna@kernel.org>
Cc: linux-nfs@vger.kernel.org
Signed-off-by: default avatarKees Cook <keescook@chromium.org>
Signed-off-by: default avatarAnna Schumaker <Anna.Schumaker@Netapp.com>
parent 121affdf

fs/nfs/client.c

+2 −2
Original line number Diff line number Diff line
@@ -280,7 +280,7 @@ EXPORT_SYMBOL_GPL(nfs_put_client); 
static struct nfs_client *nfs_match_client(const struct nfs_client_initdata *data)

{

	struct nfs_client *clp;

	const struct sockaddr *sap = data->addr;

	const struct sockaddr *sap = (struct sockaddr *)data->addr;

	struct nfs_net *nn = net_generic(data->net, nfs_net_id);

	int error;
@@ -666,7 +666,7 @@ static int nfs_init_server(struct nfs_server *server, 
	struct rpc_timeout timeparms;

	struct nfs_client_initdata cl_init = {

		.hostname = ctx->nfs_server.hostname,

		.addr = (const struct sockaddr *)&ctx->nfs_server.address,

		.addr = &ctx->nfs_server._address,

		.addrlen = ctx->nfs_server.addrlen,

		.nfs_mod = ctx->nfs_mod,

		.proto = ctx->nfs_server.protocol,

fs/nfs/dns_resolve.c

+4 −3
Original line number Diff line number Diff line
@@ -16,8 +16,9 @@ 
#include "dns_resolve.h"



ssize_t nfs_dns_resolve_name(struct net *net, char *name, size_t namelen,

		struct sockaddr *sa, size_t salen)

		struct sockaddr_storage *ss, size_t salen)

{

	struct sockaddr *sa = (struct sockaddr *)ss;

	ssize_t ret;

	char *ip_addr = NULL;

	int ip_len;
@@ -341,7 +342,7 @@ static int do_cache_lookup_wait(struct cache_detail *cd, 
}



ssize_t nfs_dns_resolve_name(struct net *net, char *name,

		size_t namelen, struct sockaddr *sa, size_t salen)

		size_t namelen, struct sockaddr_storage *ss, size_t salen)

{

	struct nfs_dns_ent key = {

		.hostname = name,
@@ -354,7 +355,7 @@ ssize_t nfs_dns_resolve_name(struct net *net, char *name, 
	ret = do_cache_lookup_wait(nn->nfs_dns_resolve, &key, &item);

	if (ret == 0) {

		if (salen >= item->addrlen) {

			memcpy(sa, &item->addr, item->addrlen);

			memcpy(ss, &item->addr, item->addrlen);

			ret = item->addrlen;

		} else

			ret = -EOVERFLOW;

fs/nfs/dns_resolve.h

+1 −1
Original line number Diff line number Diff line
@@ -32,6 +32,6 @@ extern void nfs_dns_resolver_cache_destroy(struct net *net); 
#endif



extern ssize_t nfs_dns_resolve_name(struct net *net, char *name,

		size_t namelen,	struct sockaddr *sa, size_t salen);

		size_t namelen,	struct sockaddr_storage *sa, size_t salen);



#endif

fs/nfs/fs_context.c

+7 −7
Original line number Diff line number Diff line
@@ -273,9 +273,9 @@ static const struct constant_table nfs_secflavor_tokens[] = { 
 * Address family must be initialized, and address must not be

 * the ANY address for that family.

 */

static int nfs_verify_server_address(struct sockaddr *addr)

static int nfs_verify_server_address(struct sockaddr_storage *addr)

{

	switch (addr->sa_family) {

	switch (addr->ss_family) {

	case AF_INET: {

		struct sockaddr_in *sa = (struct sockaddr_in *)addr;

		return sa->sin_addr.s_addr != htonl(INADDR_ANY);
@@ -969,7 +969,7 @@ static int nfs23_parse_monolithic(struct fs_context *fc, 
{

	struct nfs_fs_context *ctx = nfs_fc2context(fc);

	struct nfs_fh *mntfh = ctx->mntfh;

	struct sockaddr *sap = (struct sockaddr *)&ctx->nfs_server.address;

	struct sockaddr_storage *sap = &ctx->nfs_server._address;

	int extra_flags = NFS_MOUNT_LEGACY_INTERFACE;

	int ret;
@@ -1044,7 +1044,7 @@ static int nfs23_parse_monolithic(struct fs_context *fc, 
		memcpy(sap, &data->addr, sizeof(data->addr));

		ctx->nfs_server.addrlen = sizeof(data->addr);

		ctx->nfs_server.port = ntohs(data->addr.sin_port);

		if (sap->sa_family != AF_INET ||

		if (sap->ss_family != AF_INET ||

		    !nfs_verify_server_address(sap))

			goto out_no_address;
@@ -1200,7 +1200,7 @@ static int nfs4_parse_monolithic(struct fs_context *fc, 
				 struct nfs4_mount_data *data)

{

	struct nfs_fs_context *ctx = nfs_fc2context(fc);

	struct sockaddr *sap = (struct sockaddr *)&ctx->nfs_server.address;

	struct sockaddr_storage *sap = &ctx->nfs_server._address;

	int ret;

	char *c;
@@ -1314,7 +1314,7 @@ static int nfs_fs_context_validate(struct fs_context *fc) 
{

	struct nfs_fs_context *ctx = nfs_fc2context(fc);

	struct nfs_subversion *nfs_mod;

	struct sockaddr *sap = (struct sockaddr *)&ctx->nfs_server.address;

	struct sockaddr_storage *sap = &ctx->nfs_server._address;

	int max_namelen = PAGE_SIZE;

	int max_pathlen = NFS_MAXPATHLEN;

	int port = 0;
@@ -1540,7 +1540,7 @@ static int nfs_init_fs_context(struct fs_context *fc) 
		ctx->version		= nfss->nfs_client->rpc_ops->version;

		ctx->minorversion	= nfss->nfs_client->cl_minorversion;



		memcpy(&ctx->nfs_server.address, &nfss->nfs_client->cl_addr,

		memcpy(&ctx->nfs_server._address, &nfss->nfs_client->cl_addr,

			ctx->nfs_server.addrlen);



		if (fc->net_ns != net) {

fs/nfs/internal.h

+7 −7
Original line number Diff line number Diff line
@@ -69,7 +69,7 @@ static inline fmode_t flags_to_mode(int flags) 
struct nfs_client_initdata {

	unsigned long init_flags;

	const char *hostname;			/* Hostname of the server */

	const struct sockaddr *addr;		/* Address of the server */

	const struct sockaddr_storage *addr;	/* Address of the server */

	const char *nodename;			/* Hostname of the client */

	const char *ip_addr;			/* IP address of the client */

	size_t addrlen;
@@ -180,7 +180,7 @@ static inline struct nfs_fs_context *nfs_fc2context(const struct fs_context *fc) 


/* mount_clnt.c */

struct nfs_mount_request {

	struct sockaddr		*sap;

	struct sockaddr_storage	*sap;

	size_t			salen;

	char			*hostname;

	char			*dirpath;
@@ -223,7 +223,7 @@ extern void nfs4_server_set_init_caps(struct nfs_server *); 
extern struct nfs_server *nfs4_create_server(struct fs_context *);

extern struct nfs_server *nfs4_create_referral_server(struct fs_context *);

extern int nfs4_update_server(struct nfs_server *server, const char *hostname,

					struct sockaddr *sap, size_t salen,

					struct sockaddr_storage *sap, size_t salen,

					struct net *net);

extern void nfs_free_server(struct nfs_server *server);

extern struct nfs_server *nfs_clone_server(struct nfs_server *,
@@ -235,7 +235,7 @@ extern int nfs_client_init_status(const struct nfs_client *clp); 
extern int nfs_wait_client_init_complete(const struct nfs_client *clp);

extern void nfs_mark_client_ready(struct nfs_client *clp, int state);

extern struct nfs_client *nfs4_set_ds_client(struct nfs_server *mds_srv,

					     const struct sockaddr *ds_addr,

					     const struct sockaddr_storage *ds_addr,

					     int ds_addrlen, int ds_proto,

					     unsigned int ds_timeo,

					     unsigned int ds_retrans,
@@ -243,7 +243,7 @@ extern struct nfs_client *nfs4_set_ds_client(struct nfs_server *mds_srv, 
extern struct rpc_clnt *nfs4_find_or_create_ds_client(struct nfs_client *,

						struct inode *);

extern struct nfs_client *nfs3_set_ds_client(struct nfs_server *mds_srv,

			const struct sockaddr *ds_addr, int ds_addrlen,

			const struct sockaddr_storage *ds_addr, int ds_addrlen,

			int ds_proto, unsigned int ds_timeo,

			unsigned int ds_retrans);

#ifdef CONFIG_PROC_FS
@@ -894,13 +894,13 @@ static inline bool nfs_error_is_fatal_on_server(int err) 
 * Select between a default port value and a user-specified port value.

 * If a zero value is set, then autobind will be used.

 */

static inline void nfs_set_port(struct sockaddr *sap, int *port,

static inline void nfs_set_port(struct sockaddr_storage *sap, int *port,

				const unsigned short default_port)

{

	if (*port == NFS_UNSPEC_PORT)

		*port = default_port;



	rpc_set_port(sap, *port);

	rpc_set_port((struct sockaddr *)sap, *port);

}



struct nfs_direct_req {