All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff Garzik <jeff@garzik.org>
To: Colin McCabe <cmccabe@alumni.cmu.edu>
Cc: Project Hail List <hail-devel@vger.kernel.org>,
	Pete Zaitcev <zaitcev@redhat.com>
Subject: Re: [PATCHv2 1/2] cld: fix CLD_INODE_NAME_MAX woes
Date: Wed, 03 Feb 2010 17:27:29 -0500	[thread overview]
Message-ID: <4B69F851.4010808@garzik.org> (raw)
In-Reply-To: <1265204756-20122-1-git-send-email-cmccabe@alumni.cmu.edu>

On 02/03/2010 08:45 AM, Colin McCabe wrote:
> When we create a static buffer for an inode name, and treat it like a
> null-terminated string, it needs to be of length CLD_INODE_NAME_MAX + 1 so
> that it can hold the NULL-terminator.
>
> In cldc_del and cldc_open, we should check that the user-submitted inode name
> is less than or equal to CLD_INODE_NAME_MAX. Formerly we were just checking
> that it wasn't too big to fit in the packet.
>
> When copying the inode name out of struct cld_dirent_cur, use snprintf rather
> than strcpy to ensure that we never overflow the buffer. This isn't strictly
> necessary if all other checks are working perfectly, but it seems prudent.
>
> Signed-off-by: Colin McCabe<cmccabe@alumni.cmu.edu>

applied, after s/snprintf/strncpy/

In general, too, you should never pass a variable string into snprintf, 
as that may make a program vulnerable to printf format string attacks 
(user supplies "%s" as a username, for example).

A few other changes made to your XDR work:

* "\n" removed from log messages, as that is appended as needed by log 
implementation

* user_key() restored.  that is our authentication hook, and it must be 
called, even though it merely returns the username passed to it at present.

* msg type renamed back to msg op

	Jeff


  parent reply	other threads:[~2010-02-03 22:27 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-02-03 13:45 [PATCHv2 1/2] cld: fix CLD_INODE_NAME_MAX woes Colin McCabe
2010-02-03 13:45 ` [PATCHv2 2/2] cld: kill CLD_MAX_PKT_MSG, add CLD_MAX_PAYLOAD_SZ Colin McCabe
2010-02-03 23:20   ` [PATCH] " Jeff Garzik
2010-02-03 22:27 ` Jeff Garzik [this message]
2010-02-04  1:21   ` [PATCHv2 1/2] cld: fix CLD_INODE_NAME_MAX woes Colin McCabe

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=4B69F851.4010808@garzik.org \
    --to=jeff@garzik.org \
    --cc=cmccabe@alumni.cmu.edu \
    --cc=hail-devel@vger.kernel.org \
    --cc=zaitcev@redhat.com \
    /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.