Bug 1255079 - pkcon - can not confirm installation/removing component
Summary: pkcon - can not confirm installation/removing component
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: PackageKit
Version: 7.2
Hardware: ppc64
OS: Linux
urgent
urgent
Target Milestone: rc
: ---
Assignee: Richard Hughes
QA Contact: Desktop QE
URL:
Whiteboard:
Depends On:
Blocks: 1295396 1297830 1313485
TreeView+ depends on / blocked
 
Reported: 2015-08-19 14:47 UTC by Ladislav Kolacek
Modified: 2016-11-04 06:20 UTC (History)
10 users (show)

Fixed In Version: PackageKit-1.0.7-6.el7
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-11-04 06:20:00 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)
test case (1.41 KB, text/plain)
2016-02-23 14:47 UTC, Richard Hughes
no flags Details


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2016:2425 0 normal SHIPPED_LIVE PackageKit and gnome-packagekit bug fix update 2016-11-03 13:59:42 UTC

Description Ladislav Kolacek 2015-08-19 14:47:30 UTC
Description of problem:
Test case failure: https://tcms.engineering.redhat.com/case/169632/?from_plan=18766

Version-Release number of selected component (if applicable):
kernel-3.10.0-304.el7.ppc64
PackageKit-1.0.7-4.el7.ppc64

How reproducible:
100%

Steps to Reproduce:
1. As root install mc package
2. As non-root user 
      execute: 'pkcon remove gpm-libs'
3. When asked to proceed with deps removal give Yes

Actual results:
When installation asks: Proceed with changes? [N/y]
You can not confirm installation by typing y (it asks over and over in cycle). 

Expected results:
You should confirm installation by typing y and successfully finish installation.

Comment 1 Richard Hughes 2015-08-25 14:23:21 UTC
We're using the same unbuffered input code as PolicyKit, polkitagenttextlistener.c -- I'm guessing that experiences the same bug on ppc64? I assume you can't reproduce this on any other architecture?

Comment 3 Richard Hughes 2015-10-01 15:08:43 UTC
If termios.h isn't working, this points to a bigger bug with glibc. Reassigning....

Comment 9 Richard Hughes 2016-02-23 14:47:14 UTC
Created attachment 1129774 [details]
test case

Comment 10 Carlos O'Donell 2016-02-24 03:58:53 UTC
Richard,

Awesome test case. That's exactly what we need.

The code you posted is BE-unsafe, and ppc64 is a BE target.

 44                 gint c;
 45                 c = getc (tty);

This stores the value read from the tty in BE to an integer sized object.

 46                 if (c == '\n') {
 47                         /* ok, done */
 48                         break;
 49                 } else if (c == EOF) {
 50                         g_warning ("Got unexpected EOF.");
 51                         break;
 52                 } else {
 53                         g_string_append_len (str, (const gchar *) &c, 1);

The '&c' points to start of the integer sized object, which has a value of 0x0 for a character sized object e.g. { &c->[0x0], 0x0, 0x0, 0x59 'Y' }. This makes the code BE-unsafe.

To fix this requires a type conversion.

You need to do this:

                        gchar gc = (gchar) c;
                        g_string_append_len (str, (const gchar *) &gc, 1);

Knowing that `c` won't have a value of EOF, you can safely cast it to a gchar and convert the value (potentially loosing the value of the EOF, but you already checked for it earlier). Then the address of `gc` will point to the valid character value.

This is a somewhat classic BE-unsafe thing to do, which works fine on LE targets like x86_64 e.g. { &c->[0x59], 0x0, 0x0, 0x0}. This should also work fine on ppc64le.

You should audit your code for BE-unsafe operations like this since ppc64 is not going away any time soon.

Comment 11 Richard Hughes 2016-02-24 16:38:03 UTC
Hi Carlos -- thanks for the detailed report, I learned a lesson today. Thanks!

Comment 12 Matthias Clasen 2016-02-24 16:42:05 UTC
Is this sort of BE-unsafe casting not getting flagged by compiler warnings or coverity checks ?

Comment 13 Richard Hughes 2016-02-24 16:43:44 UTC
PolicyKit has the same bug so I filed it here: https://bugzilla.redhat.com/show_bug.cgi?id=1311648

Comment 14 Richard Hughes 2016-05-16 14:31:41 UTC
commit 710a9445777793e49160587882860cbb7b43e311
Author: Richard Hughes <richard>
Date:   Mon May 16 15:27:35 2016 +0100

    Make pk_console_get_prompt() big endian safe
    
    Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1255079

Comment 17 Matěj Cepl 2016-08-06 10:49:23 UTC
I could remove and reinstall gpm-libs just with pkcon. Confirmation dialog works.

Comment 19 errata-xmlrpc 2016-11-04 06:20:00 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHBA-2016-2425.html


Note You need to log in before you can comment on or make changes to this bug.