lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <83981951a40bc654c3b80960dce7a7dbcf3e0aac.camel@kernel.org>
Date: Tue, 19 Nov 2024 08:32:31 -0500
From: Jeff Layton <jlayton@...nel.org>
To: Max Kellermann <max.kellermann@...os.com>
Cc: Ilya Dryomov <idryomov@...il.com>, Patrick Donnelly
 <pdonnell@...hat.com>,  Venky Shankar <vshankar@...hat.com>,
 xiubli@...hat.com, ceph-devel@...r.kernel.org, 
 linux-kernel@...r.kernel.org, dario@...e53.de, stable@...r.kernel.org
Subject: Re: [PATCH] fs/ceph/mds_client: give up on paths longer than
 PATH_MAX

On Tue, 2024-11-19 at 14:02 +0100, Max Kellermann wrote:
> On Tue, Nov 19, 2024 at 1:51 PM Jeff Layton <jlayton@...nel.org> wrote:
> > -ENAMETOOLONG could be problematic there. This function is often called
> > when we have a dentry and need to build a path to it to send to the MDS
> > in a call. The system call that caused us to generate this path
> > probably doesn't involve a pathname itself, so the caller may be
> > confused by an -ENAMETOOLONG return.
> 
> It is unfortunate that the Ceph-MDS protocol requires having to
> convert a file descriptor back to a path name - but do you really
> believe EIO would cause less confusion? ENAMETOOLONG is exactly what
> happens, even if it's an internal error. But there are many error
> codes that describe internal errors, so there's some prior art.
> 
> EIO just doesn't fit, returning EIO would be confusing - even more so
> because EIO isn't a documented error code for open().
> 

Fair enough. EIO is just my goto when I don't have a better idea.

Probably you want some weird error code that will make people stand up
and take notice. Maybe?

     #define ENOSR           63      /* Out of streams resources */   

> If this is about building path names for sending to the MDS, and not
> for the userspace ABI, maybe the PATH_MAX limitation is wrong here. If
> Ceph doesn't have such a limitation, the Ceph code shouldn't use the
> userspace ABI limit for protocol use.
> 

It is possible to build a dir hierarchy that is deeper than can be
represented by a PATH_MAX-length string. No reason you can't allocate a
bigger buffer there. Does the MDS have a limit on the size of a path
string that it'll accept?

> > You may want to go with a more generic error code here -- -EIO or
> > something. It might also be worthwhile to leave in a pr_warn_once or
> > something since there may be users confused by this error return.
> 
> Users cannot read the kernel log, and this allows users to flood the
> kernel log. So we get all the disadvantages of the kernel log while
> our users get none of the advantages.

Sure. I'd make this a pr_warn_once or heavily ratelimit it or
something. No need for a lot of these messages, but eventually users
are going to wonder why they're getting these weird errors and bug the
admins. Giving them this hint might be helpful.

Alternately you could add a conditional tracepoint or something, but
those are more obscure.
-- 
Jeff Layton <jlayton@...nel.org>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ