From mboxrd@z Thu Jan  1 00:00:00 1970
From: Steve Grubb <sgrubb@redhat.com>
Subject: Re: [PATCH] Fix 100% CPU usage (again) due to log rotate
Date: Mon, 28 Oct 2019 18:53:44 -0400
Message-ID: <20191028185344.3e9bf236@ivy-bridge>
References: <20191025153709.2cdbaca5@gandalf.local.home>
Mime-Version: 1.0
Content-Type: text/plain; charset=WINDOWS-1252
Content-Transfer-Encoding: quoted-printable
Return-path: <linux-audit-bounces@redhat.com>
In-Reply-To: <20191025153709.2cdbaca5@gandalf.local.home>
List-Unsubscribe: <https://www.redhat.com/mailman/options/linux-audit>,
	<mailto:linux-audit-request@redhat.com?subject=unsubscribe>
List-Archive: <https://www.redhat.com/archives/linux-audit>
List-Post: <mailto:linux-audit@redhat.com>
List-Help: <mailto:linux-audit-request@redhat.com?subject=help>
List-Subscribe: <https://www.redhat.com/mailman/listinfo/linux-audit>,
	<mailto:linux-audit-request@redhat.com?subject=subscribe>
Sender: linux-audit-bounces@redhat.com
Errors-To: linux-audit-bounces@redhat.com
To: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Cc: Srikantha Munipemmareddy <smunipemmareddy@vmware.com>, Soundararajan <ravi@vmware.com>, Clark Williams <williams@redhat.com>, Stepan Broz <sbroz@starbug.usersys.redhat.com>, Srivatsa, Tanmay Nag <Tanmay@vmware.com>, Darren Hart <dvhart@vmware.com>, Ravi, Joseph Zuk <jzuk@vmware.com>, linux-audit@redhat.com, Bhat <srivatsab@vmware.com>, Loi Tran <loi@vmware.com>
List-Id: linux-audit@redhat.com

On Fri, 25 Oct 2019 15:37:09 -0400
"Steven Rostedt (VMware)" <rostedt@goodmis.org> wrote:

> If the num_logs is set to 0 and keep_logs is set, we go back into a
> loop of MAX_INT! in rotate_logs().

Thanks. Applied.

-Steve

> commit 9145e97c ("Do not rotate logs when num_logs < 2.") fixed the
> issue with not going further if num_logs is less than 2, because if
> num_logs is zero, we trigger this bug because of the loop:
>=20
>   for (i=3Dnum_logs - 1; i>1; i--) {
>=20
> As i is an unsigned int, if num_logs is zero, we initialize i to
> "0 - 1" or 4,294,967,295. Thus, runs this loop over 4 billion times!
>=20
> But this caused a regressing if keep_logs is set, so this
> num_logs < 2 was skipped if keep_logs is set, causing the huge loop
> to run once again if num_logs is zero!
>=20
> There's no reason i needs to be unsigned, if i is signed, then if we
> pass in num_logs =3D 0, then i will start off as -1, and -1 > 1 will
> fail the loop and fix the issue. I don't envision anyone wanting to
> keep 2,147,483,648 log files around.
>=20
> Fixes: a7f9f8b5 ("Fix auditd regression where keep_logs is limited by
> rotate_logs 2 file test") Signed-off-by: Steven Rostedt (VMware)
> <rostedt@goodmis.org> ---
> diff --git a/src/auditd-event.c b/src/auditd-event.c
> index 1c93173..68eacd5 100644
> --- a/src/auditd-event.c
> +++ b/src/auditd-event.c
> @@ -1012,8 +1012,8 @@ static void fix_disk_permissions(void)
>  =20
>  static void rotate_logs(unsigned int num_logs, unsigned int
> keep_logs) {
> -=09int rc;
> -=09unsigned int len, i;
> +=09int rc, i;
> +=09unsigned int len;
>  =09char *oldname, *newname;
> =20
>  =09/* Check that log rotation is enabled in the configuration
> file. There @@ -1065,7 +1065,7 @@ static void rotate_logs(unsigned
> int num_logs, unsigned int keep_logs) snprintf(oldname, len, "%s.1",
> config->log_file);=20
>  =09known_logs =3D 0;
> -=09for (i=3Dnum_logs - 1; i>1; i--) {
> +=09for (i=3D(int)num_logs - 1; i>1; i--) {
>  =09=09snprintf(oldname, len, "%s.%u", config->log_file,
> i-1); snprintf(newname, len, "%s.%u", config->log_file, i);
>  =09=09/* if the old file exists */