| 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
| ||
|
Message-ID: <e39846e70cf60611400827614278895104cc03be.camel@ibm.com>
Date: Mon, 17 Nov 2025 19:42:25 +0000
From: Viacheslav Dubeyko <Slava.Dubeyko@....com>
To: "409411716@....tku.edu.tw" <409411716@....tku.edu.tw>
CC: "david.laight.linux@...il.com" <david.laight.linux@...il.com>,
Xiubo Li
<xiubli@...hat.com>, "sagi@...mberg.me" <sagi@...mberg.me>,
"idryomov@...il.com" <idryomov@...il.com>,
"linux-nvme@...ts.infradead.org"
<linux-nvme@...ts.infradead.org>,
"linux-fscrypt@...r.kernel.org"
<linux-fscrypt@...r.kernel.org>,
"kbusch@...nel.org" <kbusch@...nel.org>,
"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"ebiggers@...nel.org" <ebiggers@...nel.org>,
"visitorckw@...il.com"
<visitorckw@...il.com>,
"hch@....de" <hch@....de>,
"home7438072@...il.com"
<home7438072@...il.com>,
"axboe@...nel.dk" <axboe@...nel.dk>, "tytso@....edu"
<tytso@....edu>,
"andriy.shevchenko@...el.com" <andriy.shevchenko@...el.com>,
"jaegeuk@...nel.org" <jaegeuk@...nel.org>,
"ceph-devel@...r.kernel.org"
<ceph-devel@...r.kernel.org>
Subject: RE: [PATCH v5 6/6] ceph: replace local base64 helpers with lib/base64
On Sun, 2025-11-16 at 18:36 +0800, Guan-Chun Wu wrote:
> On Fri, Nov 14, 2025 at 06:07:26PM +0000, Viacheslav Dubeyko wrote:
> > On Fri, 2025-11-14 at 14:02 +0800, Guan-Chun Wu wrote:
> > > Remove the ceph_base64_encode() and ceph_base64_decode() functions and
> > > replace their usage with the generic base64_encode() and base64_decode()
> > > helpers from lib/base64.
> > >
> > > This eliminates the custom implementation in Ceph, reduces code
> > > duplication, and relies on the shared Base64 code in lib.
> > > The helpers preserve RFC 3501-compliant Base64 encoding without padding,
> > > so there are no functional changes.
> > >
> > > This change also improves performance: encoding is about 2.7x faster and
> > > decoding achieves 43-52x speedups compared to the previous local
> > > implementation.
> > >
> > > Reviewed-by: Kuan-Wei Chiu <visitorckw@...il.com>
> > > Signed-off-by: Guan-Chun Wu <409411716@....tku.edu.tw>
> > > ---
> > > fs/ceph/crypto.c | 60 ++++--------------------------------------------
> > > fs/ceph/crypto.h | 6 +----
> > > fs/ceph/dir.c | 5 ++--
> > > fs/ceph/inode.c | 2 +-
> > > 4 files changed, 9 insertions(+), 64 deletions(-)
> > >
> > > diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
> > > index 7026e794813c..b6016dcffbb6 100644
> > > --- a/fs/ceph/crypto.c
> > > +++ b/fs/ceph/crypto.c
> > > @@ -15,59 +15,6 @@
> > > #include "mds_client.h"
> > > #include "crypto.h"
> > >
> > > -/*
> > > - * The base64url encoding used by fscrypt includes the '_' character, which may
> > > - * cause problems in snapshot names (which can not start with '_'). Thus, we
> > > - * used the base64 encoding defined for IMAP mailbox names (RFC 3501) instead,
> > > - * which replaces '-' and '_' by '+' and ','.
> > > - */
> > > -static const char base64_table[65] =
> > > - "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+,";
> > > -
> > > -int ceph_base64_encode(const u8 *src, int srclen, char *dst)
> > > -{
> > > - u32 ac = 0;
> > > - int bits = 0;
> > > - int i;
> > > - char *cp = dst;
> > > -
> > > - for (i = 0; i < srclen; i++) {
> > > - ac = (ac << 8) | src[i];
> > > - bits += 8;
> > > - do {
> > > - bits -= 6;
> > > - *cp++ = base64_table[(ac >> bits) & 0x3f];
> > > - } while (bits >= 6);
> > > - }
> > > - if (bits)
> > > - *cp++ = base64_table[(ac << (6 - bits)) & 0x3f];
> > > - return cp - dst;
> > > -}
> > > -
> > > -int ceph_base64_decode(const char *src, int srclen, u8 *dst)
> > > -{
> > > - u32 ac = 0;
> > > - int bits = 0;
> > > - int i;
> > > - u8 *bp = dst;
> > > -
> > > - for (i = 0; i < srclen; i++) {
> > > - const char *p = strchr(base64_table, src[i]);
> > > -
> > > - if (p == NULL || src[i] == 0)
> > > - return -1;
> > > - ac = (ac << 6) | (p - base64_table);
> > > - bits += 6;
> > > - if (bits >= 8) {
> > > - bits -= 8;
> > > - *bp++ = (u8)(ac >> bits);
> > > - }
> > > - }
> > > - if (ac & ((1 << bits) - 1))
> > > - return -1;
> > > - return bp - dst;
> > > -}
> > > -
> > > static int ceph_crypt_get_context(struct inode *inode, void *ctx, size_t len)
> > > {
> > > struct ceph_inode_info *ci = ceph_inode(inode);
> > > @@ -318,7 +265,7 @@ int ceph_encode_encrypted_dname(struct inode *parent, char *buf, int elen)
> > > }
> > >
> > > /* base64 encode the encrypted name */
> > > - elen = ceph_base64_encode(cryptbuf, len, p);
> > > + elen = base64_encode(cryptbuf, len, p, false, BASE64_IMAP);
> > > doutc(cl, "base64-encoded ciphertext name = %.*s\n", elen, p);
> > >
> > > /* To understand the 240 limit, see CEPH_NOHASH_NAME_MAX comments */
> > > @@ -412,7 +359,8 @@ int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
> > > tname = &_tname;
> > > }
> > >
> > > - declen = ceph_base64_decode(name, name_len, tname->name);
> > > + declen = base64_decode(name, name_len,
> > > + tname->name, false, BASE64_IMAP);
> > > if (declen <= 0) {
> > > ret = -EIO;
> > > goto out;
> > > @@ -426,7 +374,7 @@ int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
> > >
> > > ret = fscrypt_fname_disk_to_usr(dir, 0, 0, &iname, oname);
> > > if (!ret && (dir != fname->dir)) {
> > > - char tmp_buf[CEPH_BASE64_CHARS(NAME_MAX)];
> > > + char tmp_buf[BASE64_CHARS(NAME_MAX)];
> > >
> > > name_len = snprintf(tmp_buf, sizeof(tmp_buf), "_%.*s_%ld",
> > > oname->len, oname->name, dir->i_ino);
> > > diff --git a/fs/ceph/crypto.h b/fs/ceph/crypto.h
> > > index 23612b2e9837..b748e2060bc9 100644
> > > --- a/fs/ceph/crypto.h
> > > +++ b/fs/ceph/crypto.h
> > > @@ -8,6 +8,7 @@
> > >
> > > #include <crypto/sha2.h>
> > > #include <linux/fscrypt.h>
> > > +#include <linux/base64.h>
> > >
> > > #define CEPH_FSCRYPT_BLOCK_SHIFT 12
> > > #define CEPH_FSCRYPT_BLOCK_SIZE (_AC(1, UL) << CEPH_FSCRYPT_BLOCK_SHIFT)
> > > @@ -89,11 +90,6 @@ static inline u32 ceph_fscrypt_auth_len(struct ceph_fscrypt_auth *fa)
> > > */
> > > #define CEPH_NOHASH_NAME_MAX (180 - SHA256_DIGEST_SIZE)
> > >
> > > -#define CEPH_BASE64_CHARS(nbytes) DIV_ROUND_UP((nbytes) * 4, 3)
> > > -
> > > -int ceph_base64_encode(const u8 *src, int srclen, char *dst);
> > > -int ceph_base64_decode(const char *src, int srclen, u8 *dst);
> > > -
> > > void ceph_fscrypt_set_ops(struct super_block *sb);
> > >
> > > void ceph_fscrypt_free_dummy_policy(struct ceph_fs_client *fsc);
> > > diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> > > index d18c0eaef9b7..0fa7c7777242 100644
> > > --- a/fs/ceph/dir.c
> > > +++ b/fs/ceph/dir.c
> > > @@ -998,13 +998,14 @@ static int prep_encrypted_symlink_target(struct ceph_mds_request *req,
> > > if (err)
> > > goto out;
> > >
> > > - req->r_path2 = kmalloc(CEPH_BASE64_CHARS(osd_link.len) + 1, GFP_KERNEL);
> > > + req->r_path2 = kmalloc(BASE64_CHARS(osd_link.len) + 1, GFP_KERNEL);
> > > if (!req->r_path2) {
> > > err = -ENOMEM;
> > > goto out;
> > > }
> > >
> > > - len = ceph_base64_encode(osd_link.name, osd_link.len, req->r_path2);
> > > + len = base64_encode(osd_link.name, osd_link.len,
> > > + req->r_path2, false, BASE64_IMAP);
> > > req->r_path2[len] = '\0';
> > > out:
> > > fscrypt_fname_free_buffer(&osd_link);
> > > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> > > index a6e260d9e420..b691343cb7f1 100644
> > > --- a/fs/ceph/inode.c
> > > +++ b/fs/ceph/inode.c
> > > @@ -958,7 +958,7 @@ static int decode_encrypted_symlink(struct ceph_mds_client *mdsc,
> > > if (!sym)
> > > return -ENOMEM;
> > >
> > > - declen = ceph_base64_decode(encsym, enclen, sym);
> > > + declen = base64_decode(encsym, enclen, sym, false, BASE64_IMAP);
> > > if (declen < 0) {
> > > pr_err_client(cl,
> > > "can't decode symlink (%d). Content: %.*s\n",
> >
> > Looks good!
> >
> > Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@....com>
> >
> > Have you run xfstests for this patchset?
>
> Hi Slava,
>
> Thanks for the review.
>
> I haven't run xfstests on this patchset yet.
>
>
I have run the xfstests for CephFS with applied patchset. I don't see any new
issues. We had failures with generic/452 generic/639 before applying the
patchset. So, as far as I can see, patchset works well.
Tested-by: Viacheslav Dubeyko <Slava.Dubeyko@....com>
Thanks,
Slava.
Powered by blists - more mailing lists