All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: amir73il@gmail.com
Cc: linux-unionfs@vger.kernel.org
Subject: [bug report] ovl: make sure that real fid is 32bit aligned in memory
Date: Tue, 5 May 2020 16:50:26 +0300	[thread overview]
Message-ID: <20200505135026.GA38935@mwanda> (raw)

Hello Amir Goldstein,

The patch cbe7fba8edfc: "ovl: make sure that real fid is 32bit
aligned in memory" from Nov 15, 2019, leads to the following static
checker warning:

	fs/overlayfs/export.c:791 ovl_fid_to_fh()
	warn: check that subtract can't underflow

fs/overlayfs/export.c
   775  static struct ovl_fh *ovl_fid_to_fh(struct fid *fid, int buflen, int fh_type)
   776  {
   777          struct ovl_fh *fh;
   778  
   779          /* If on-wire inner fid is aligned - nothing to do */
   780          if (fh_type == OVL_FILEID_V1)
   781                  return (struct ovl_fh *)fid;
   782  
   783          if (fh_type != OVL_FILEID_V0)
   784                  return ERR_PTR(-EINVAL);
   785  
   786          fh = kzalloc(buflen, GFP_KERNEL);
   787          if (!fh)
   788                  return ERR_PTR(-ENOMEM);
   789  
   790          /* Copy unaligned inner fh into aligned buffer */
   791          memcpy(&fh->fb, fid, buflen - OVL_FH_WIRE_OFFSET);
                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^

   792          return fh;
   793  }

Samtch thinks buflen can be "0,4-128".  OVL_FH_WIRE_OFFSET is 3. The
problem is that 0 - 3 is a negative and the memcpy() will crash.

In do_handle_to_path() we do:

	handle_dwords = handle->handle_bytes >> 2;

Handle ->handle_bytes is non-zero but when it's >> 2 then it can become
zero.  It's a round down.  In ovl_fh_to_dentry() we do:

	int len = fh_len << 2;

If we rounded down to zero then "len" is still zero.  Obviously one fix
would be to add a check in ovl_fid_to_fh().

	if (buflen < sizeof(*fh))
		return ERR_PTR(-EINVAL);

But that feels like papering over the bug.

regards,
dan carpenter

             reply	other threads:[~2020-05-05 13:50 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-05 13:50 Dan Carpenter [this message]
2020-05-05 16:13 ` [bug report] ovl: make sure that real fid is 32bit aligned in memory Amir Goldstein
2020-05-05 18:07   ` [PATCH] ovl: potential crash in ovl_fid_to_fh() Dan Carpenter
2020-05-05 18:07     ` Dan Carpenter
2020-05-05 18:15     ` Amir Goldstein
2020-05-05 18:15       ` Amir Goldstein
2020-05-05 18:33       ` [PATCH v2] " Dan Carpenter
2020-05-05 18:33         ` Dan Carpenter
2020-05-05 18:08   ` [bug report] ovl: make sure that real fid is 32bit aligned in memory Dan Carpenter
  -- strict thread matches above, loose matches on Subject: below --
2019-12-13 10:37 Dan Carpenter
2019-12-13 12:22 ` Miklos Szeredi
2019-12-13 12:58   ` Dan Carpenter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200505135026.GA38935@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=amir73il@gmail.com \
    --cc=linux-unionfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.