Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Steven Rostedt <rostedt@goodmis.org>
CC:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        David Wysochanski <dwysocha@redhat.com>,
        Bruce Fields <bfields@fieldses.org>
Subject: Re: [PATCH v2 01/25] NFSD: Fix TP_printk() format specifier in
 trace_nfsd_dirent()
Thread-Topic: [PATCH v2 01/25] NFSD: Fix TP_printk() format specifier in
 trace_nfsd_dirent()
Thread-Index: AQHXR0uQjwrq7dbjxka4bUuLuYA4jKrgD64AgAFwTwCAAAWZAIAAO4wAgAAE14CAAAITAIAAAo6AgAAMMYA=
Date:   Thu, 13 May 2021 20:01:00 +0000
Message-ID: <5BC8482E-1902-432A-B162-774A96E4C2B7@oracle.com>
References: <162083366966.3108.12581818416105328952.stgit@klimt.1015granger.net>
 <162083370248.3108.7424008399973918267.stgit@klimt.1015granger.net>
 <20210512122623.79ee0dda@gandalf.local.home>
 <238C0E2D-C2A4-4578-ADD2-C565B3B99842@oracle.com>
 <20210513105018.7539996a@gandalf.local.home>
 <C3D7DA41-C5B1-4388-B55C-E8A1280E9C9E@oracle.com>
 <107A51EE-E0A8-46FE-9E62-9FC586B91F19@oracle.com>
 <20210513150047.6b8ed9fb@gandalf.local.home>
 <C618B795-ED20-4BA8-9C18-333EB42AD9CD@oracle.com>
 <20210513151721.02a7fdd1@gandalf.local.home>
In-Reply-To: <20210513151721.02a7fdd1@gandalf.local.home>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-ID: <E838509F28585F459B9D2A44CE0957CA@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 8a202859-12d8-4891-59b7-08d91649d463
X-MS-Exchange-CrossTenant-originalarrivaltime: 13 May 2021 20:01:00.0434
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: LqbDexOdhLHbN6yo/1c3dnrO0n77mr1yKQaiYz7lry0sXa09HbDXUzwIZSDlrcgx8nATC8QE1IrzdatcUFKMgw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR10MB4306
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9983 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 suspectscore=0
 malwarescore=0 spamscore=0 mlxscore=0 adultscore=0 bulkscore=0
 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2104190000 definitions=main-2105130139
X-Proofpoint-ORIG-GUID: WsC75f0U1fr_jZ1mFpBuNYEKWaX2xLPp
X-Proofpoint-GUID: WsC75f0U1fr_jZ1mFpBuNYEKWaX2xLPp
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9983 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 impostorscore=0
 adultscore=0 spamscore=0 clxscore=1015 mlxlogscore=999 bulkscore=0
 mlxscore=0 priorityscore=1501 malwarescore=0 lowpriorityscore=0
 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2104190000 definitions=main-2105130140
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On May 13, 2021, at 3:17 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
>=20
> On Thu, 13 May 2021 19:08:13 +0000
> Chuck Lever III <chuck.lever@oracle.com> wrote:
>=20
>> The tracepoints that currently use '%.*s' no longer work when
>> using "trace-cmd start/stop/show". They were working before
>> 9a6944fee68e, so I consider this a regression. I plan to
>> submit patches to address this for 5.13-rc. I guess they will
>> have to go without the use of the new _len macros for now,
>> and you can push the macros in v5.14.
>=20
> That's a separate bug. I'm currently running this patch through my tests,
> and will push to Linus when it completes. Feel free to test this one too.

Confirmed that applying the below patch addresses the regression.

Tested-by: Chuck Lever <chuck.lever@oracle.com>

Thanks for your quick response!


> -- Steve
>=20
> From eb01f5353bdaa59600b29d864819056a0e3de24d Mon Sep 17 00:00:00 2001
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> Date: Thu, 13 May 2021 12:23:24 -0400
> Subject: [PATCH] tracing: Handle %.*s in trace_check_vprintf()
>=20
> If a trace event uses the %*.s notation, the trace_check_vprintf() will
> fail and will warn about a bad processing of strings, because it does not
> take into account the length field when processing the star (*) part.
> Have it handle this case as well.
>=20
> Link: https://lore.kernel.org/linux-nfs/238C0E2D-C2A4-4578-ADD2-C565B3B99=
842@oracle.com/
>=20
> Reported-by: Chuck Lever III <chuck.lever@oracle.com>
> Fixes: 9a6944fee68e2 ("tracing: Add a verifier to check string pointers f=
or trace events")
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
> kernel/trace/trace.c | 31 +++++++++++++++++++++++++++----
> 1 file changed, 27 insertions(+), 4 deletions(-)
>=20
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 560e4c8d3825..a21ef9cd2aae 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -3704,6 +3704,9 @@ void trace_check_vprintf(struct trace_iterator *ite=
r, const char *fmt,
> 		goto print;
>=20
> 	while (*p) {
> +		bool star =3D false;
> +		int len =3D 0;
> +
> 		j =3D 0;
>=20
> 		/* We only care about %s and variants */
> @@ -3725,13 +3728,17 @@ void trace_check_vprintf(struct trace_iterator *i=
ter, const char *fmt,
> 				/* Need to test cases like %08.*s */
> 				for (j =3D 1; p[i+j]; j++) {
> 					if (isdigit(p[i+j]) ||
> -					    p[i+j] =3D=3D '*' ||
> 					    p[i+j] =3D=3D '.')
> 						continue;
> +					if (p[i+j] =3D=3D '*') {
> +						star =3D true;
> +						continue;
> +					}
> 					break;
> 				}
> 				if (p[i+j] =3D=3D 's')
> 					break;
> +				star =3D false;
> 			}
> 			j =3D 0;
> 		}
> @@ -3744,6 +3751,9 @@ void trace_check_vprintf(struct trace_iterator *ite=
r, const char *fmt,
> 		iter->fmt[i] =3D '\0';
> 		trace_seq_vprintf(&iter->seq, iter->fmt, ap);
>=20
> +		if (star)
> +			len =3D va_arg(ap, int);
> +
> 		/* The ap now points to the string data of the %s */
> 		str =3D va_arg(ap, const char *);
>=20
> @@ -3762,8 +3772,18 @@ void trace_check_vprintf(struct trace_iterator *it=
er, const char *fmt,
> 			int ret;
>=20
> 			/* Try to safely read the string */
> -			ret =3D strncpy_from_kernel_nofault(iter->fmt, str,
> -							  iter->fmt_size);
> +			if (star) {
> +				if (len + 1 > iter->fmt_size)
> +					len =3D iter->fmt_size - 1;
> +				if (len < 0)
> +					len =3D 0;
> +				ret =3D copy_from_kernel_nofault(iter->fmt, str, len);
> +				iter->fmt[len] =3D 0;
> +				star =3D false;
> +			} else {
> +				ret =3D strncpy_from_kernel_nofault(iter->fmt, str,
> +								  iter->fmt_size);
> +			}
> 			if (ret < 0)
> 				trace_seq_printf(&iter->seq, "(0x%px)", str);
> 			else
> @@ -3775,7 +3795,10 @@ void trace_check_vprintf(struct trace_iterator *it=
er, const char *fmt,
> 			strncpy(iter->fmt, p + i, j + 1);
> 			iter->fmt[j+1] =3D '\0';
> 		}
> -		trace_seq_printf(&iter->seq, iter->fmt, str);
> +		if (star)
> +			trace_seq_printf(&iter->seq, iter->fmt, len, str);
> +		else
> +			trace_seq_printf(&iter->seq, iter->fmt, str);
>=20
> 		p +=3D i + j + 1;
> 	}
> --=20
> 2.29.2
>=20

--
Chuck Lever