From: Andreas Dilger Subject: Re: [PATCH] fscrypto: make filename crypto functions return 0 on success Date: Thu, 15 Sep 2016 03:50:47 -0600 Message-ID: <44B2D8F1-56B4-4F87-909C-F00E3EBA4074@dilger.ca> References: <1473894045-65215-1-git-send-email-ebiggers@google.com> Mime-Version: 1.0 (Mac OS X Mail 9.3 \(3124\)) Content-Type: multipart/signed; boundary="Apple-Mail=_771F2F00-39EC-4007-B6FE-B0F89F74EC7B"; protocol="application/pgp-signature"; micalg=pgp-sha256 Cc: linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, tytso@mit.edu, jaegeuk@kernel.org To: Eric Biggers Return-path: In-Reply-To: <1473894045-65215-1-git-send-email-ebiggers@google.com> Sender: linux-fsdevel-owner@vger.kernel.org List-Id: linux-ext4.vger.kernel.org --Apple-Mail=_771F2F00-39EC-4007-B6FE-B0F89F74EC7B Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii On Sep 14, 2016, at 5:00 PM, Eric Biggers wrote: >=20 > Several filename crypto functions: fname_decrypt(), > fscrypt_fname_disk_to_usr(), and fscrypt_fname_usr_to_disk(), returned > the output length on success or -errno on failure. However, the = output > length was redundant with the value written to 'oname->len'. It is = also > potentially error-prone to make callers have to check for '< 0' = instead > of '!=3D 0'. >=20 > Therefore, make these functions return 0 instead of a length, and make > the callers who cared about the return value being a length use > 'oname->len' instead. >=20 > This change also fixes the inconsistency of fname_encrypt() actually > already returning 0 on success, not a length like the other filename > crypto functions and as documented in its function comment. >=20 > Signed-off-by: Eric Biggers Reviewed-by: Andreas Dilger > --- > fs/crypto/fname.c | 52 = ++++++++++++++++++++++++++++++---------------------- > fs/ext4/dir.c | 5 +++-- > fs/ext4/symlink.c | 5 ++--- > fs/f2fs/namei.c | 4 ++-- > 4 files changed, 37 insertions(+), 29 deletions(-) >=20 > diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c > index 6bbc3b1..49fd564 100644 > --- a/fs/crypto/fname.c > +++ b/fs/crypto/fname.c > @@ -35,11 +35,11 @@ static void fname_crypt_complete(struct = crypto_async_request *req, int res) > } >=20 > /** > - * fname_encrypt() - > + * fname_encrypt() - encrypt a filename > * > - * This function encrypts the input filename, and returns the length = of the > - * ciphertext. Errors are returned as negative numbers. We trust the = caller to > - * allocate sufficient memory to oname string. > + * The caller must have allocated sufficient memory for the @oname = string. > + * > + * Return: 0 on success, -errno on failure > */ > static int fname_encrypt(struct inode *inode, > const struct qstr *iname, struct fscrypt_str = *oname) > @@ -105,20 +105,22 @@ static int fname_encrypt(struct inode *inode, > } > kfree(alloc_buf); > skcipher_request_free(req); > - if (res < 0) > + if (res < 0) { > printk_ratelimited(KERN_ERR > "%s: Error (error code %d)\n", __func__, = res); > + return res; > + } >=20 > oname->len =3D ciphertext_len; > - return res; > + return 0; > } >=20 > -/* > - * fname_decrypt() > - * This function decrypts the input filename, and returns > - * the length of the plaintext. > - * Errors are returned as negative numbers. > - * We trust the caller to allocate sufficient memory to oname = string. > +/** > + * fname_decrypt() - decrypt a filename > + * > + * The caller must have allocated sufficient memory for the @oname = string. > + * > + * Return: 0 on success, -errno on failure > */ > static int fname_decrypt(struct inode *inode, > const struct fscrypt_str *iname, > @@ -168,7 +170,7 @@ static int fname_decrypt(struct inode *inode, > } >=20 > oname->len =3D strnlen(oname->name, iname->len); > - return oname->len; > + return 0; > } >=20 > static const char *lookup_table =3D > @@ -279,6 +281,10 @@ EXPORT_SYMBOL(fscrypt_fname_free_buffer); > /** > * fscrypt_fname_disk_to_usr() - converts a filename from disk space = to user > * space > + * > + * The caller must have allocated sufficient memory for the @oname = string. > + * > + * Return: 0 on success, -errno on failure > */ > int fscrypt_fname_disk_to_usr(struct inode *inode, > u32 hash, u32 minor_hash, > @@ -287,13 +293,12 @@ int fscrypt_fname_disk_to_usr(struct inode = *inode, > { > const struct qstr qname =3D FSTR_TO_QSTR(iname); > char buf[24]; > - int ret; >=20 > if (fscrypt_is_dot_dotdot(&qname)) { > oname->name[0] =3D '.'; > oname->name[iname->len - 1] =3D '.'; > oname->len =3D iname->len; > - return oname->len; > + return 0; > } >=20 > if (iname->len < FS_CRYPTO_BLOCK_SIZE) > @@ -303,9 +308,9 @@ int fscrypt_fname_disk_to_usr(struct inode *inode, > return fname_decrypt(inode, iname, oname); >=20 > if (iname->len <=3D FS_FNAME_CRYPTO_DIGEST_SIZE) { > - ret =3D digest_encode(iname->name, iname->len, = oname->name); > - oname->len =3D ret; > - return ret; > + oname->len =3D digest_encode(iname->name, iname->len, > + oname->name); > + return 0; > } > if (hash) { > memcpy(buf, &hash, 4); > @@ -315,15 +320,18 @@ int fscrypt_fname_disk_to_usr(struct inode = *inode, > } > memcpy(buf + 8, iname->name + iname->len - 16, 16); > oname->name[0] =3D '_'; > - ret =3D digest_encode(buf, 24, oname->name + 1); > - oname->len =3D ret + 1; > - return ret + 1; > + oname->len =3D 1 + digest_encode(buf, 24, oname->name + 1); > + return 0; > } > EXPORT_SYMBOL(fscrypt_fname_disk_to_usr); >=20 > /** > * fscrypt_fname_usr_to_disk() - converts a filename from user space = to disk > * space > + * > + * The caller must have allocated sufficient memory for the @oname = string. > + * > + * Return: 0 on success, -errno on failure > */ > int fscrypt_fname_usr_to_disk(struct inode *inode, > const struct qstr *iname, > @@ -333,7 +341,7 @@ int fscrypt_fname_usr_to_disk(struct inode *inode, > oname->name[0] =3D '.'; > oname->name[iname->len - 1] =3D '.'; > oname->len =3D iname->len; > - return oname->len; > + return 0; > } > if (inode->i_crypt_info) > return fname_encrypt(inode, iname, oname); > diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c > index 67415e0..4d4b910 100644 > --- a/fs/ext4/dir.c > +++ b/fs/ext4/dir.c > @@ -260,11 +260,12 @@ static int ext4_readdir(struct file *file, = struct dir_context *ctx) > /* Directory is encrypted */ > err =3D = fscrypt_fname_disk_to_usr(inode, > 0, 0, &de_name, &fstr); > + de_name =3D fstr; > fstr.len =3D save_len; > - if (err < 0) > + if (err) > goto errout; > if (!dir_emit(ctx, > - fstr.name, err, > + de_name.name, de_name.len, > le32_to_cpu(de->inode), > get_dtype(sb, = de->file_type))) > goto done; > diff --git a/fs/ext4/symlink.c b/fs/ext4/symlink.c > index 4d83d9e..55aaf30 100644 > --- a/fs/ext4/symlink.c > +++ b/fs/ext4/symlink.c > @@ -67,14 +67,13 @@ static const char *ext4_encrypted_get_link(struct = dentry *dentry, > goto errout; >=20 > res =3D fscrypt_fname_disk_to_usr(inode, 0, 0, &cstr, &pstr); > - if (res < 0) > + if (res) > goto errout; >=20 > paddr =3D pstr.name; >=20 > /* Null-terminate the name */ > - if (res <=3D pstr.len) > - paddr[res] =3D '\0'; > + paddr[pstr.len] =3D '\0'; > if (cpage) > put_page(cpage); > set_delayed_call(done, kfree_link, paddr); > diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c > index 73fa356..a5468c2 100644 > --- a/fs/f2fs/namei.c > +++ b/fs/f2fs/namei.c > @@ -1048,7 +1048,7 @@ static const char = *f2fs_encrypted_get_link(struct dentry *dentry, > goto errout; >=20 > res =3D fscrypt_fname_disk_to_usr(inode, 0, 0, &cstr, &pstr); > - if (res < 0) > + if (res) > goto errout; >=20 > /* this is broken symlink case */ > @@ -1060,7 +1060,7 @@ static const char = *f2fs_encrypted_get_link(struct dentry *dentry, > paddr =3D pstr.name; >=20 > /* Null-terminate the name */ > - paddr[res] =3D '\0'; > + paddr[pstr.len] =3D '\0'; >=20 > put_page(cpage); > set_delayed_call(done, kfree_link, paddr); > -- > 2.8.0.rc3.226.g39d4020 >=20 > -- > To unsubscribe from this list: send the line "unsubscribe linux-ext4" = in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Cheers, Andreas --Apple-Mail=_771F2F00-39EC-4007-B6FE-B0F89F74EC7B Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename=signature.asc Content-Type: application/pgp-signature; name=signature.asc Content-Description: Message signed with OpenPGP using GPGMail -----BEGIN PGP SIGNATURE----- Comment: GPGTools - http://gpgtools.org iQIVAwUBV9pu+HKl2rkXzB/gAQjxfhAAoqVxK9bV29mudcZ8zBXfwUj0xBd8N5lP HwtWX40Mi60qOiICnw8Bo/a6EoMzc6hMxow5TijwAAqU/zhfEPYByjpk2UTdW/K6 3njpMLgRCowA8vArUte15ROMBiJoB903e8L8HiEPkJHkC8iNzzKCY6795mrbNsDT TUSbnHoSONy9HZZ8jJBTsaRtnZm0zU6dTtIKyUXRyCL1x/3E/RiOOQNFWdReZP3b XiSN4ojDJ1T+7xvVVUt6i5hTIa3dkTtBLEtlJIFCZzprgprV3bqXN+0hb4/J2qGd YKTZx+8lf5PaJXDlv6ArSD+c9ZEQRy3I+lbzDRXkH/cSyAAUdQskoKFFnWPi7JaM SBziAYRRLMGzk3SgQtUHPqMozfqI3WmPCZLrd1oobz+7UkhidAQ/C/BgV+j2ePlu HDDspfFOjfM3sxSH6dJREDqIM+9y1A6NJkRCbftLGWjzZaSmpeQu9uoT1N+qIHF9 w7qvz54w/4O/JP+Mcks/63FODfzSA+Aw7GwEkiEIRh7XO9UrRNXZJTe+du+KXgDj JUtqGhfwXg+L8FwqookA+sLjUTYyO3aMa+iVv47xsiCdIA+6dFYmM/HgyPrTfmdX tBDR6neBgt7xJPwOi1L/aV2FUR2C9rCJEBshEStnzHOVraAsfgYQ/bYPX0LUUuG/ YHYmr55LyQ0= =AMDn -----END PGP SIGNATURE----- --Apple-Mail=_771F2F00-39EC-4007-B6FE-B0F89F74EC7B--