Re: [Evolution-hackers] [patch] segfault in gal_a11y_e_cell_popup_new

[Thomas Mittelstaedt <tmstaedt t-mittelstaedt de>]

Am Dienstag, den 01.11.2011, 07:04 +0100 schrieb Milan Crha:
> On Mon, 2011-10-31 at 21:22 +0100, Thomas Mittelstaedt wrote:
> > Just had a segfault in gal_a11y_e_cell_popup_new. Turned out that
> > the cast
> > 	popupcell=  E_CELL_POPUP (cell_view->ecell);
> > 
> > would turn up a broken pointer, crashing afterward.
> 
> 	Hi,
> it depends on the brokenness kind, if either the cell_view is already
> freed, or the cell_view->ecell is pointing to already freed memory. In
> both cases you are trying to access maybe-overwritten memory and read
> from it, which can do pretty much anything.
> 
> > I inserted the following on my side:
> > 
> > 	ECellPopup *popupcell = NULL;
> > 	ECellView* child_view = NULL;
> > 
> > 	if (E_IS_CELL_POPUP(cell_view->ecell)) {
> > 		popupcell = E_CELL_POPUP(cell_view->ecell);
> > 	}
> 
> That it didn't crash for you is probably just a coincidence, that the
> memory (allocated on GSlice) wasn't overwritten yet. You can check with
> valgrind, using command like this:
>    $ G_SLICE=always-malloc valgrind --num-callers=50 evolution &>log.txt
> 
> I suppose yours "Just had a segfault" also means that you do not face it
> every day, it just happened today, thus you do not have a reproducer for
> this?

You are right. I just had another crash with the above code changes. gdb
told me that 
popupcell->popup_cell_view->cell_view.ecell was a broken pointer and
popupcell->popup_cell_view->cell_view.e_table_model was 0. So, I
inserted another "sanity check". Let's see if it crashes again.




-- 
thomas

Insert check to prevent crash

From: Thomas Mittelstaedt <tmstaedt t-mittelstaedt de>


---

 a11y/e-table/gal-a11y-e-cell-popup.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)


diff --git a/a11y/e-table/gal-a11y-e-cell-popup.c b/a11y/e-table/gal-a11y-e-cell-popup.c
index 141ce17..b5583fa 100644
--- a/a11y/e-table/gal-a11y-e-cell-popup.c
+++ b/a11y/e-table/gal-a11y-e-cell-popup.c
@@ -89,14 +89,19 @@ gal_a11y_e_cell_popup_new (ETableItem *item,
 {
 	AtkObject *a11y;
 	GalA11yECell *cell;
-	ECellPopup *popupcell;
+	ECellPopup *popupcell = NULL;
 	ECellView* child_view = NULL;
 
-	popupcell=  E_CELL_POPUP(cell_view->ecell);
+	if (E_IS_CELL_POPUP(cell_view->ecell)) {
+		popupcell = E_CELL_POPUP(cell_view->ecell);
+	}
+	
+	if (popupcell && popupcell->popup_cell_view &&
+			popupcell->popup_cell_view->cell_view.e_table_model) {
 
-	if (popupcell && popupcell->popup_cell_view)
 		child_view = popupcell->popup_cell_view->child_view;
-
+	}
+	
 	if (child_view && child_view->ecell) {
 		a11y = gal_a11y_e_cell_registry_get_object (NULL,
 							    item,