Message-id: <1363573173.14386.70.camel@kjgkr>
Subject: Re: [PATCH 5/5] f2fs: avoid BUG_ON from check_nid_range and update
 return path in do_read_inode
From: Jaegeuk Kim <jaegeuk.kim@samsung.com>
Reply-to: jaegeuk.kim@samsung.com
To: Namjae Jeon <linkinjeon@gmail.com>
Cc: linux-f2fs-devel@lists.sourceforge.net, linux-fsdevel@vger.kernel.org,
        linux-kernel@vger.kernel.org, Namjae Jeon <namjae.jeon@samsung.com>,
        Amit Sahrawat <a.sahrawat@samsung.com>
Date: Mon, 18 Mar 2013 11:19:33 +0900
In-reply-to: <1363508840-2954-1-git-send-email-linkinjeon@gmail.com>
References: <1363508840-2954-1-git-send-email-linkinjeon@gmail.com>
Organization: samsung
Content-type: multipart/signed; micalg=pgp-sha1;
 protocol="application/pgp-signature"; boundary="=-QkQQr4AHcuugn1CMuaLa"
MIME-version: 1.0
DLP-Filter: Pass
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3348
Lines: 97


--=-QkQQr4AHcuugn1CMuaLa
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

2013-03-17 (=EC=9D=BC), 17:27 +0900, Namjae Jeon:
> From: Namjae Jeon <namjae.jeon@samsung.com>
>=20
> In function check_nid_range, there is no need to trigger BUG_ON and make =
kernel stop.
> Instead it could just check and indicate the inode number to be EINVAL.
> Update the return path in do_read_inode to use the return from check_nid_=
range.
>=20
> Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
> Signed-off-by: Amit Sahrawat <a.sahrawat@samsung.com>
> ---
>  fs/f2fs/f2fs.h  |    6 ++++--
>  fs/f2fs/inode.c |    6 +++++-
>  2 files changed, 9 insertions(+), 3 deletions(-)
>=20
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index be7ae70..1dae921 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -515,9 +515,11 @@ static inline void mutex_unlock_op(struct f2fs_sb_in=
fo *sbi, enum lock_type t)
>  /*
>   * Check whether the given nid is within node id range.
>   */
> -static inline void check_nid_range(struct f2fs_sb_info *sbi, nid_t nid)
> +static inline int check_nid_range(struct f2fs_sb_info *sbi, nid_t nid)
>  {
> -	BUG_ON((nid >=3D NM_I(sbi)->max_nid));
> +	if (nid >=3D NM_I(sbi)->max_nid)
> +		return -EINVAL;
> +	return 0;

At this moment, I'd like to apply this patch and remain BUG_ON together
since we should find real bugs in f2fs.
How do you think?

>  }
> =20
>  #define F2FS_DEFAULT_ALLOCATED_BLOCKS	1
> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> index ddae412..6d82020 100644
> --- a/fs/f2fs/inode.c
> +++ b/fs/f2fs/inode.c
> @@ -44,7 +44,11 @@ static int do_read_inode(struct inode *inode)
>  	struct f2fs_inode *ri;
> =20
>  	/* Check if ino is within scope */
> -	check_nid_range(sbi, inode->i_ino);
> +	if (check_nid_range(sbi, inode->i_ino)) {
> +		f2fs_msg(inode->i_sb, KERN_ERR, "bad inode number: %lu",
> +			 (unsigned long) inode->i_ino);
> +		return -EINVAL;
> +	}
> =20
>  	node_page =3D get_node_page(sbi, inode->i_ino);
>  	if (IS_ERR(node_page))

--=20
Jaegeuk Kim
Samsung

--=-QkQQr4AHcuugn1CMuaLa
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: This is a digitally signed message part
Content-Transfer-Encoding: 7bit

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)

iQIcBAABAgAGBQJRRnm1AAoJEEAUqH6CSFDSTTwP/1TczyKSxKCyXNEkiXDcBR/v
qacKxpOXLxLAJGth9EdF/fhK2mBkWQej73CGHNK/Uc+Nq4PPYltX81QhuqRP7wWg
LO86JgnQFO8yaR87jY1G4H2FdD8j0+cf3ZSkAAuiJMt17VP3K4J5W+31MxD5JGcb
Luc0Pb/Ol6EEt6BmoVDx03QINK5NeZBfZJoK/TzZS3xVFh7jHza48/Lqvh1I/e8/
mJOfl3x+ObfR9qfpNTDmSlijEPCuQ6Che1sE6kboaVA8RwGSHsIDpTAKI82yNBN/
V8lvbB23kHL9Cx0ZHTZREphm6mdWVtIOJcURON9SEkg3EnP+E/2h4fkN3pYvX8zI
qpCIk+2GBdhPmPOXTUdPXcdYxheAp/S4e6TpVvMrMh0O3wf7vJv9dcDGVK8Hd1Zu
Ur22L7nnMvgfWshBYjJZCPlkCnP6x9gw5hgckGl2d9fkrkaokMaF/4gSCJn2rCph
7wzwOslvSmI4hLUxmm0dS6nSO4FC3oo9a1+RGVjIGChX+RbQ5kIiP33sLLAPfb+n
uXegLySIU8BdngKK7rQSfAbjPTxYXccilYPgwuCVL6FT69pLRAi0l8V9e4ETQ0Uk
ZzTHfL0Qlws8QMDlwvGqiCPmRQ4eCXlTRHN3CdGrurROXZeKgyt7BspD3Wb1GVmw
o6g9qr5FKpRg+YT4SV/p
=2oRq
-----END PGP SIGNATURE-----

--=-QkQQr4AHcuugn1CMuaLa--

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/