linux-kernel - Re: [ 32/62] kdb,vt_console: Fix missed data due to pager overruns

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20121019213904.GB13292@decadent.org.uk>
Date:	Fri, 19 Oct 2012 22:39:05 +0100
From:	Ben Hutchings <ben@...adent.org.uk>
To:	Jason Wessel <jason.wessel@...driver.com>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-kernel@...r.kernel.org, stable@...r.kernel.org,
	alan@...rguk.ukuu.org.uk
Subject: Re: [ 32/62] kdb,vt_console: Fix missed data due to pager overruns

On Thu, Oct 18, 2012 at 07:45:00PM -0700, Greg Kroah-Hartman wrote:
> 3.4-stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Jason Wessel <jason.wessel@...driver.com>
> 
> commit 17b572e82032bc246324ce136696656b66d4e3f1 upstream.
[...]
> This also means that the vt screen needs to set the kdb COLUMNS
> variable.
[...]
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -3475,6 +3475,19 @@ int con_debug_enter(struct vc_data *vc)
>  			kdb_set(2, setargs);
>  		}
>  	}
> +	if (vc->vc_cols < 999) {
> +		int colcount;
> +		char cols[4];
> +		const char *setargs[3] = {
> +			"set",
> +			"COLUMNS",
> +			cols,
> +		};
> +		if (kdbgetintenv(setargs[0], &colcount)) {
> +			snprintf(cols, 4, "%i", vc->vc_cols);
> +			kdb_set(2, setargs);
> +		}
> +	}

Presumably the idea here was to avoid overriding the 'COLUMNS'
variable if it is already set, but it's actually testing whether the
variable 'set' is set.  (Besides which, there should be a kdbsetenv()
instead of this gross hack of faking up a user command.  And whyever
did kdb have to make argc off-by-one from the usual C convention?)

[...]
> +	if (KDB_STATE(PAGER)) {
> +		/*
> +		 * Check printed string to decide how to bump the
> +		 * kdb_nextline to control when the more prompt should
> +		 * show up.
> +		 */
> +		int got = 0;
> +		len = retlen;
> +		while (len--) {
> +			if (kdb_buffer[len] == '\n') {
> +				kdb_nextline++;
> +				got = 0;
> +			} else if (kdb_buffer[len] == '\r') {
> +				got = 0;
> +			} else {
> +				got++;
> +			}
> +		}
> +		kdb_nextline += got / (colcount + 1);

What if more than one logical line is wrapped?
Why divide by colcount + 1?  Shouldn't it be (got - 1) / colcount?
 
Ben.

-- 
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
                                                              - Albert Camus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/